Skip to content

Make the API more flexible (allow to ommit vizier: when crossmatching with a vizier catalog)#3194

Merged
bsipocz merged 2 commits into
astropy:mainfrom
cds-astro:fix-xmatch-error-message-on-wrong-catalog-name
Jan 30, 2025
Merged

Make the API more flexible (allow to ommit vizier: when crossmatching with a vizier catalog)#3194
bsipocz merged 2 commits into
astropy:mainfrom
cds-astro:fix-xmatch-error-message-on-wrong-catalog-name

Conversation

@ManonMarchand

Copy link
Copy Markdown
Member

While trying to make the error message clearer as suggested in comment #3191 (comment) I figured that we can also add the vizier: if the user did not write it. We already test if the table is available anyway, so we have the information.

@codecov

codecov Bot commented Jan 29, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.34%. Comparing base (46ca592) to head (6b0af57).
Report is 328 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3194      +/-   ##
==========================================
+ Coverage   68.18%   68.34%   +0.16%     
==========================================
  Files         231      231              
  Lines       19048    19190     +142     
==========================================
+ Hits        12987    13116     +129     
- Misses       6061     6074      +13     

☔ View full report in Codecov by Sentry.
📢 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.

@ManonMarchand

ManonMarchand commented Jan 29, 2025

Copy link
Copy Markdown
Member Author

Arg windows and their strange return lines. I'm checking how to fix the test.

EDIT: did a dirty little fix, if you have a better way, I'll take it

@ManonMarchand ManonMarchand force-pushed the fix-xmatch-error-message-on-wrong-catalog-name branch from 130e01c to 1f3ce5f Compare January 29, 2025 14:40
this makes the API less strict to use
@ManonMarchand ManonMarchand force-pushed the fix-xmatch-error-message-on-wrong-catalog-name branch from 1f3ce5f to 6b0af57 Compare January 29, 2025 14:41

@keflavich keflavich 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 great.

The documentation actually does include vizier:, but I just glazed right over it when reading the docs.

We could do with some more examples to show, e.g., cone search with Vizier catalogs, but that can go in a different PR (just reminding you here since you noted it in #3191)

@ManonMarchand

ManonMarchand commented Jan 29, 2025

Copy link
Copy Markdown
Member Author

Yes, I plan a PR on documentation in the next months, but it's a bit longer to write than fixing the error message/ making this small quality of life improvement. Let's keep #3191 open 🙂 (and maybe we can change the title so that it does not look too bad for Xmatch)

@bsipocz bsipocz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

@bsipocz bsipocz merged commit ade38dc into astropy:main Jan 30, 2025
@bsipocz bsipocz added this to the v0.4.10 milestone Jan 30, 2025
@ManonMarchand ManonMarchand deleted the fix-xmatch-error-message-on-wrong-catalog-name branch January 30, 2025 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants