Export plugin: table support

[kecnry]

Description

This pull request adds support for exporting plugin tables via the new export plugin.

Up for discussion:

• how should these tables be labeled? Here I have {plugin_name}:{table_name} where table_name defaults to table. This adds flexibility for multiple tables per-plugin, but does NOT support multiple plugin instances of the same plugin (i.e. creating an independent instance of model fitting after app initialization will not be registered here). We probably will want to follow the same convention for plugin plots, in that case we know that we have multiple plugin plots in at least the line profile plugin in Imviz.
• are we ready to expose helper.plugin_tables or should we keep that private for now?
• do we want a follow-up effort for consistent handling of checking and overwriting an existing filename?

Screen.Recording.2024-03-13.at.4.17.10.PM.mov

TODO before ready for review:

• snackbar for success/failures
• test coverage
• documentation

Change log entry

• Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
  list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
  should add a no-changelog-entry-needed label.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

• Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
• Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
• Do the proposed changes follow the STScI Style Guides?
• Are tests added/updated as required? If so, do they follow the STScI Style Guides?
• Are docs added/updated as required? If so, do they follow the STScI Style Guides?
• Did the CI pass? If not, are the failures related?
• Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone. Bugfix milestone also needs an accompanying backport label.
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

[codecov]

Codecov Report

Attention: Patch coverage is 87.05882% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 88.72%. Comparing base (adda177) to head (7d846e4).
Report is 6 commits behind head on main.

❗ Current head 7d846e4 differs from pull request most recent head 05a63ae. Consider uploading reports for the commit 05a63ae to get more accurate results

Files	Patch %	Lines
jdaviz/core/user_api.py	25.00%	6 Missing ⚠️
jdaviz/configs/default/plugins/export/export.py	82.60%	4 Missing ⚠️
jdaviz/app.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2755      +/-   ##
==========================================
+ Coverage   88.63%   88.72%   +0.09%     
==========================================
  Files         108      108              
  Lines       16210    16267      +57     
==========================================
+ Hits        14367    14433      +66     
+ Misses       1843     1834       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cshanahan1]

Reviewed it and works well, just a few comments.

What do you think about having 'imviz_export' not be the default filename, but rather update it based on what is selected in the plugin?

I also agree that we need to figure out how to handle overwriting, and it should work the same way for all exportable things. In my export subset PR, I have it overwrite by default with no warning. This could be a follow up?

[kecnry]

What do you think about having 'imviz_export' not be the default filename, but rather update it based on what is selected in the plugin?

I thought about that - I think we could do something similar to data labels where the code provides a default, but once you override then it stays stuck. I'm just not sure if that's worth it depending on when we decide to use solara and raise a save file dialog every time anyways. I can create a ticket since this applies to the entire export plugin and we can decide whether to implement it or postpone 🐱

I also agree that we need to figure out how to handle overwriting, and it should work the same way for all exportable things. In my export subset PR, I have it overwrite by default with no warning. This could be a follow up?

Ok, agreed, I'll create that ticket as well 🐱 (again - might not be worth it if we're replacing with the file dialog soon).

[kecnry]

I had updated it to this. Cubeviz does have it's own page for movies - we may want to refactor that around to all point to the same place (the rest point to Imviz docs) as we build out more feature.

[rosteen]

Sounds like overwrite behavior is a separate effort. It seemed like this didn't save out the file the first time I clicked, but I couldn't replicated and the rest of my export attempts worked, so maybe I'm crazy 🤷. I think the file extension needs to be handled a little better but other than that this looks good.

[rosteen]

I would prefer to check that the filename ends with a supported format string rather than checking for a . anywhere in the filename. Trying to save with a filename like "table.export" leads to a snackbar error.

[kecnry]

do you know how to access a list of supported extensions?

[rosteen]

The snackbar message lists some formats but I don't know where to access that list.

[kecnry]

ok, I'll investigate and can always hardcode a small list otherwise or probably should just have a format dropdown like we have for viewer exporting.

[kecnry]

see the latest commit - there were quite a few, so I just hardcoded ecsv, csv, fits as format options and append the extension if it does not match.

[rosteen]

Works for me, thanks!