Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: adding list_collections to IRSA #2952

Merged
merged 4 commits into from
Feb 17, 2024

Conversation

bsipocz
Copy link
Member

@bsipocz bsipocz commented Feb 16, 2024

This is to close #2940

note: the query may take long right now, but we'll cache the VOTABLE format for this query to work around astropy/pyvo#524

@bsipocz bsipocz added this to the v0.4.7 milestone Feb 16, 2024
Copy link

codecov bot commented Feb 16, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (893eae2) 66.81% compared to head (f55b116) 66.80%.

Files Patch % Lines
astroquery/ipac/irsa/core.py 25.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2952      +/-   ##
==========================================
- Coverage   66.81%   66.80%   -0.01%     
==========================================
  Files         237      237              
  Lines       18316    18320       +4     
==========================================
+ Hits        12237    12238       +1     
- Misses       6079     6082       +3     

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

@bsipocz
Copy link
Member Author

bsipocz commented Feb 16, 2024

Hmm, hold back on this, cadc has a get_collections which returns a dictionary, maybe we should do that here, too? I'll investigate what we do cache on server side, it maybe more pragmatic to only have the quick results in a list_collections rather than waiting for a longer run for something like get_collections

cc @andamian to comment on the API plans.

@bsipocz
Copy link
Member Author

bsipocz commented Feb 16, 2024

Hmm, actally the cadc approach doesn't pull in an actual description, so I'm inclined towards merging this PR as is and have list_collections along with list_catalogs for IRSA. (actual descriptions we do have in the registry for these collections, but I definitely don't want to do a registry search for query_sia or for this module just right now)

@bsipocz bsipocz merged commit 1c6b7bf into astropy:main Feb 17, 2024
8 of 10 checks passed
@bsipocz bsipocz deleted the ENH_irsa_list_collection branch April 6, 2024 03:19
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.

ENH: have a way to list collections for query_sia
1 participant