Skip to content

Roman catalog detection#4216

Open
jbchampagne wants to merge 25 commits into
spacetelescope:mainfrom
jbchampagne:roman-catalog-detection
Open

Roman catalog detection#4216
jbchampagne wants to merge 25 commits into
spacetelescope:mainfrom
jbchampagne:roman-catalog-detection

Conversation

@jbchampagne

@jbchampagne jbchampagne commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

This PR updates the CatalogImporter to detect ra/dec/x/y columns when there are multiple columns that satisfy the requirements to be coordinates, such as 'ra', 'ra_centroid', 'ra_err', etc. This is in preparation for Roman source catalogs which will include various quantities from the source extraction procedure, including positional uncertainties. Uncertainty or min/max columns should be excluded. The intended behavior is to default to the first (non-excluded) match. Added test coverage for input catalogs with multiple coordinate options.

Fixes #JDAT-6141

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

@jbchampagne jbchampagne added this to the 5.1 milestone Jun 3, 2026
@jbchampagne jbchampagne requested a review from rosteen as a code owner June 3, 2026 16:17
@jbchampagne jbchampagne added the plugin Label for plugins common to multiple configurations label Jun 3, 2026
Comment thread jdaviz/utils.py
'infrared', 'fraction', 'gradient', 'ratio',
'integrated,' 'radian', 'random', 'parallax', 'range',
'decade', 'decadal', 'decrement', 'deconvolve']
'decade', 'decadal', 'decrement', 'deconvolve',

@cshanahan1 cshanahan1 Jun 3, 2026

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.

i think we may not want to exclude 'psf' here, because source positions may come from psf fitting (*whoops meant to leave this comment on the line below)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good point, removed

Comment thread CHANGES.rst Outdated
@jbchampagne jbchampagne force-pushed the roman-catalog-detection branch from f9e7280 to 04cf746 Compare June 4, 2026 20:00
@codecov

codecov Bot commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.15%. Comparing base (960b6e8) to head (a58c97c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4216      +/-   ##
==========================================
- Coverage   85.16%   85.15%   -0.02%     
==========================================
  Files         229      229              
  Lines       31208    31219      +11     
==========================================
+ Hits        26579    26583       +4     
- Misses       4629     4636       +7     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread jdaviz/core/loaders/importers/catalog/test_catalog.py
@jbchampagne jbchampagne requested a review from cshanahan1 June 5, 2026 17:08

@cshanahan1 cshanahan1 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.

looks good, approved pending confirmation that all test failures are known issues

@jbchampagne jbchampagne force-pushed the roman-catalog-detection branch from 4a9f2ff to e6e638a Compare June 11, 2026 16:14
jbchampagne and others added 9 commits June 15, 2026 11:40
# Conflicts:
#	CHANGES.rst
* Add clarifying hint about table query toggle

* Add toggle to limit results to science type files

* Add changelog

* Only filter type if it's in the table

* Only show science filter for astroquery

* Add option back to user API
# Conflicts:
#	CHANGES.rst
Co-authored-by: Matthew Portman <29168957+MatthewPortman@users.noreply.github.com>
@jbchampagne jbchampagne force-pushed the roman-catalog-detection branch from e3d7619 to f8bbdb4 Compare June 15, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin Label for plugins common to multiple configurations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants