Skip to content

Astroquery (SDSS & Gaia) resolver#3906

Merged
kecnry merged 26 commits into
spacetelescope:mainfrom
kecnry:resolver-astroquery
Dec 1, 2025
Merged

Astroquery (SDSS & Gaia) resolver#3906
kecnry merged 26 commits into
spacetelescope:mainfrom
kecnry:resolver-astroquery

Conversation

@kecnry

@kecnry kecnry commented Nov 21, 2025

Copy link
Copy Markdown
Member

Description

This pull request achieves feature parity with the catalogs plugin from loaders by implementing a basic astroquery resolver, supporting SDSS and Gaia cone search only, that can feed tables into the catalog resolver (and therefore currently requires the app.state.catalogs_in_dc = True).

Screen.Recording.2025-11-21.at.12.31.31.PM.mov

After this (and removing the dev-flag), the only functionality from catalogs plugin that is not yet implemented are (both planned for upcoming table work):

  • zoom-to-selected in table
  • highlight selected in table and syncing with clicked marker from image viewer

For this PR, should we put the entire resolver under the catalogs dev-flag since it currently can't load anything else? #3907 could then remove the dev-flag but exclude SDSS/Gaia without the flag set?

Potential future follow-up:

  • other telescopes/missions (including MAST)
  • allow querying for non catalogs?
  • (created ticket) debug why sometimes there is an error snackbar about failing to add to image viewer (when it seems successful) with traceback 'BqplotImageLayerArtist' object has no attribute 'density_mark' - seems unrelated to this PR and likely affects all catalogs

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?
  • If new remote data is added that uses MAST, is the URI added to the cache-download.yml workflow?
  • 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)?

@kecnry kecnry added this to the 4.5 milestone Nov 21, 2025
@kecnry kecnry added the no-changelog-entry-needed changelog bot directive label Nov 21, 2025
Comment on lines +337 to +348
exceptions = []
for viewer_label in viewer_select.selected:
viewer = self.app._jdaviz_helper.viewers.get(viewer_label)
try:
viewer.data_menu.add_data(data_label)
except Exception:
except Exception as e:
failed_viewers.append(viewer_label)
exceptions.append(str(e))
if len(failed_viewers) > 0:
msg = f"Failed to add {data_label} to viewers: {', '.join(failed_viewers)}"
self.app.hub.broadcast(SnackbarMessage(msg, sender=self, color='error'))
self.app.hub.broadcast(SnackbarMessage(msg, sender=self, color='error',
traceback=exceptions))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this was to help debug a snackbar error that ended up being no directly related - ticket to debug/fix has been created, but this is still a useful addition

Comment on lines 37 to +52
@@ -50,6 +43,13 @@ class VOResolver(BaseResolver):
radius_unit_items = List().tag(sync=True)
radius_unit_selected = Unicode("deg").tag(sync=True)

waveband_items = List().tag(sync=True)
waveband_selected = Any().tag(sync=True) # Any to accept Nonetype
resource_filter_coverage = Bool(False).tag(sync=True)
resource_items = List([]).tag(sync=True)
resource_selected = Any().tag(sync=True) # Any to accept Nonetype
resources_loading = Bool(False).tag(sync=True)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

just re-ordering to match the UI a little better and be consistent with ordering in the new resolver

@kecnry kecnry mentioned this pull request Nov 21, 2025
13 tasks
@kecnry kecnry force-pushed the resolver-astroquery branch from 7220377 to 55ec9a6 Compare November 25, 2025 15:24
Comment thread jdaviz/core/loaders/resolvers/astroquery/astroquery.vue Outdated
Comment thread jdaviz/core/loaders/tests/test_load_catalogs.py

@MatthewPortman MatthewPortman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code looks good but when no results are returned from the query, I get 'No compatible importer found.' I'm not sure if it's in the scope of this ticket but I would prefer that warning to be specific in this case.

To reproduce, I used the GAIA coordinates from the video for and SDSS query.

@kecnry

kecnry commented Nov 26, 2025

Copy link
Copy Markdown
Member Author

I can probably add an alert similar to the one when the results exceed the max results - let me see if I can get that in this afternoon.

EDIT: turned out to be very straightforward, pushing soon.

@kecnry

kecnry commented Nov 26, 2025

Copy link
Copy Markdown
Member Author

not sure where the sudden test failure came from - will investigate monday morning

@kecnry kecnry force-pushed the resolver-astroquery branch from faebafd to bcb0820 Compare December 1, 2025 14:58
@kecnry kecnry merged commit dd0146b into spacetelescope:main Dec 1, 2025
16 of 19 checks passed
@kecnry kecnry deleted the resolver-astroquery branch December 1, 2025 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants