Skip to content

FIREFLY-1623: Improve python API for tables #64

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

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

jaladh-singhal
Copy link
Member

@jaladh-singhal jaladh-singhal commented Dec 11, 2024

Fixes FIREFLY-1623

  • Added url param to show_table() that can be used instead of file_on_server or target_search_info - this is similar to show_fits() which accepts url param as part of additonal params
  • Documented usage of url param for both table and image
  • Created two methods for sorting/filtering a loaded table, with documentation

Bonus: removed internal classes from API reference because they are of no use to end-user and were causing confusion

Note: I wanted to create a 3rd method to set UI options too but it's more complicated so I deferred it to FIREFLY-1631 in the interest of time

Testing

Copy the method examples (added in rst docs) in your notebook with firefly_client installed, they should work

@jaladh-singhal jaladh-singhal self-assigned this Dec 11, 2024
Copy link
Collaborator

@stargaser stargaser left a comment

Choose a reason for hiding this comment

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

The changes look good to me. I tried them in a notebook. I left a couple of suggestions inline.

Do the changes warrant a bumping of the minor version?

In the course of testing, I found that table overlays did not work at all in Safari 17.6, but they worked as expected in Chrome.

@jaladh-singhal
Copy link
Member Author

Do the changes warrant a bumping of the minor version?

@stargaser yes, we will make a new minor release soon for the workshop

Copy link
Contributor

@robyww robyww left a comment

Choose a reason for hiding this comment

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

Looks good!

@jaladh-singhal jaladh-singhal merged commit 1019f72 into master Dec 12, 2024
@jaladh-singhal jaladh-singhal deleted the FIREFLY-1623-tbl-api branch December 12, 2024 21:17
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.

3 participants