Catalogs plugin results table

[kcarver1]

Description

This pull request is to add a table of results to the catalogs plugin in Imviz.

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)?

[kcarver1]

Here is what the table currently looks like in the UI:

Displaying the table in a new window:

[kecnry]

Happy to see that the diff isn't too big, nice work! Just have a few suggestions from skimming the code and then once they're addressed I'll do some local tests.

There are also a bunch of codestyle failures, if you scroll down to the section below and click "Details" on the "CI / Code style checks (pull_request)" row (see screenshot) you can see the list of them and address them.

[rosteen]

I left one suggestion for the documentation. Additionally, the UI in my testing doesn't seem to be sizing the table and associated elements correctly:

You can see in the first screenshot that it's overflowing the space on the left and right, and in the second screenshot when I expand the plugin, some of the words end up overlapping. One of us can help you tweak the .vue file to try to improve this.

[rosteen]

The UI looks good now, looks like the failing test is because the table column names in the catalog file are not "ra" and "dec" - I think they're something like "sky_centroid.ra" instead.

[kcarver1]

The UI looks good now, looks like the failing test is because the table column names in the catalog file are not "ra" and "dec" - I think they're something like "sky_centroid.ra" instead.

Yes, this works! I pushed those changes. Do you have any ideas on how to also access the object id? I talked to Cami this morning about it and the example catalog she showed me used 'label' but that fails the tests.

[kcarver1]

What should happen when a user searches through the SDSS catalog and then tries to search through their own file? Should the table automatically be cleared?

[rosteen]

What should happen when a user searches through the SDSS catalog and then tries to search through their own file? Should the table automatically be cleared?

That's a good question. I think my first instinct is to append to the existing table, since there's a clear button if the user does actually want to clear their results.

Edit: athough maybe in that case we would want a "source" column in the table to show where each row came from 🤔

[kecnry]

I would say for now the table should do whatever behavior currently exists for the markers so that they relate directly to each other.

[codecov]

Codecov Report

Attention: Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.

Project coverage is 88.73%. Comparing base (07cc0f3) to head (a662b76).
Report is 187 commits behind head on main.

Files with missing lines	Patch %	Lines
jdaviz/configs/imviz/plugins/catalogs/catalogs.py	83.33%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2915      +/-   ##
==========================================
- Coverage   88.76%   88.73%   -0.04%     
==========================================
  Files         111      111              
  Lines       17172    17262      +90     
==========================================
+ Hits        15243    15317      +74     
- Misses       1929     1945      +16     

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

[kecnry]

This is looking great! Just two suggestions to consider, but I think they could both be deferred to the follow-up effort if you'd rather.

[kecnry]

you can always do something like:

'Object ID': row.get('label', '')

which will then use the "label" column if it exists, and otherwise will fill that entry with an empty string.

[kecnry]

instead of repeating these if-statements, can we move this logic into the previous if/elif statements and reference the object directly instead of having to rely on self.app._catalog_source_table?

Or would SkyCoordTable which is populated for each case and then used for the markers be able to take the object IDs as well, in which case we wouldn't need separate cases at all and would also benefit from being able to act on filtered_skycoord_table in case any entries have been removed (otherwise we'll have to deal with this later when implementing click-to-zoom functionality since some entries might be missing marks)?

[kcarver1]

Hey Kyle, I moved the code into the original if/elif statements. Could you further explain what you mean when you say "reference the object directly"? Are you referring to query_region_result?

[kecnry]

Awesome thanks! I just meant that you could use skycoord_table and table instead of self.app._catalog_source_table since I'm guessing we'll need to get rid of that eventually, but that really doesn't matter and I can see arguments both ways - feel free to leave it as is now!

[rosteen]

Looks good to me - you can let my approval stand if you do end up taking Kyle's suggestions and he approves.

[kecnry]

Looks good to me now - thanks for all your hard work!