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

update vizier keywords lists #2825

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

ManonMarchand
Copy link
Member

Hi all 🙂

This PR updates the keywords list for VizieR.

It modifies the json files and also includes a tiny script to renew them more often. I can remove this from the PR if this is not a good place for such a script.

As a result, there will be no errors for the new missions and objects (like "Exoplanets" or "Gravitational_wave").

New objects and missions are pretty rare, but is there a way to implement a renewal every few weeks/months? Maybe linked to the releases of astroquery?

@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Merging #2825 (c032334) into main (51316d7) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2825   +/-   ##
=======================================
  Coverage   66.40%   66.40%           
=======================================
  Files         235      235           
  Lines       18140    18140           
=======================================
  Hits        12046    12046           
  Misses       6094     6094           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

📢 Have feedback on the report? Share it here.

@keflavich
Copy link
Contributor

I like the idea of regularly updating. Maybe we can ask the CDS team to push a notification whenever they update the keywords?

@bsipocz bsipocz added the vizier label Sep 7, 2023
inverse_dict.json Outdated Show resolved Hide resolved
@bsipocz
Copy link
Member

bsipocz commented Sep 7, 2023

I wonder whether it even makes sense to keep these files around, and rather just have a method or function that runs the TAP query? That's what we do in other modules, and given these are not really big data in any sense, it shouldn't be controversial to have the up-to-date version from the server rather than a bundled version.

@ManonMarchand
Copy link
Member Author

Hello, thanks for the feedback 🙂

For now, we can already update the files with this PR.

I asked the team, and the keywords list changes less than two times a year, and we'd rather avoid an additional query on our side for something this static. Moreover, there is an ongoing change: we'll soon follow the UAT keywords, meaning that the current list will become legacy (still supported but will never change again).

So the solution I'm offering for now is that I'll manually do the update when needed, and the keywords support will be refactored when the UAT changes comes to production.

Do you think it makes sense?

@bsipocz
Copy link
Member

bsipocz commented Sep 11, 2023

@ManonMarchand - Thanks for the explanation. When an archival preference is known, we definitely like to follow it, so let's have the update merged in.

@bsipocz bsipocz added this to the v0.4.7 milestone Sep 11, 2023
@bsipocz bsipocz merged commit 5891b94 into astropy:main Sep 11, 2023
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