Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some helpers for working with result tables #536

Merged
merged 1 commit into from
Apr 5, 2024
Merged

Conversation

jeremykubica
Copy link
Contributor

This is a temporary fix to enable more dynamic exploration until we address #535. It adds an index column to the table resulting from ResultList.to_table() which can be used to map back to the result's index in the original ResultList. It also adds a helper function to keep the two in sync (which is a bit of a hack and should be removed by a more proper approach like #535).

Adds examples of how to use the new functionality to the results notebook.

@jeremykubica jeremykubica requested a review from wilsonbb April 5, 2024 13:26
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Collaborator

@wilsonbb wilsonbb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +904 to +905
self.filter_results(table["index"], "Table filtered")
table["index"] = range(len(table))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a comment for when we actually address #535 : I'm presuming we want to actually set an astropy.table index, but curious if there are any specific reasons why we were choosing not to do so (for example extra performance cost not worth it for hopefully small ResultLists)

@jeremykubica jeremykubica merged commit 605b716 into main Apr 5, 2024
2 checks passed
@jeremykubica jeremykubica deleted the demo_fixes branch April 5, 2024 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants