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

Switch to dict instead of ordered dict in Vizier find_catalogs #3023

Open
ManonMarchand opened this issue Jun 6, 2024 · 3 comments
Open
Labels

Comments

@ManonMarchand
Copy link
Member

Since astroquery is for python > 3.10, I was wondering if you'd be on board to switch the result of Vizier.find_tables to normal dictionaries here:

return OrderedDict([(R.name, R) for R in vo_tree.resources])

I was thinking something like:

return {R.name: R.description for R in vo_tree.resources}

And I guess the changes to the API would be minimal, but we'd have a nicer repr. Or is there a specific reason that I missed for this?

Also,could we return the description directly instead of the astropy.io.votable.tree.Resource ? The only other information that I think might be useful would be the number of rows in the catalog.

@keflavich
Copy link
Contributor

I'm onboard with both changes.

@bsipocz
Copy link
Member

bsipocz commented Jun 7, 2024

Yeap, both sounds reasonable.

@bsipocz bsipocz added vizier and removed question labels Jun 7, 2024
@ManonMarchand
Copy link
Member Author

So, this motivated #3028 which will make find_catalogs way faster. However, switching to a dictionary will remove the possibility to do this (in the docs here):

from astroquery.vizier import Vizier
catalog_list = Vizier.find_catalogs('Kang W51')
catalogs = Vizier.get_catalogs(catalog_list.values())
print(catalogs)

Where we use values instead of keys. So that's actually a change of API. Maybe not such a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants