Maintenance for the VizieR module

ManonMarchand commented 5 months ago

Hi astroquery,

What the PR does

This is just a maintenance PR. It does:

allow the tests to run in the documentation
switch from -meta.all to -meta in find_catalogs (the extended information is not in the <RESOURCE> tag anyway, so it is not used by the method). This increases the speed of this method and reduces the size of the downloaded information a lot
update the remote tests with examples that return less results to make them a bit faster, and reduce the impact on VizieR's servers

Internally, to use the -meta option that has no value (a weird thing of the ASU protocol) I had to change the post request in find_catalog. This has the added benefit that we can now use the -obsolete option rather than filtering afterwards.

There is no change to the API.

On the ucd test failure

This is also there on main. It is an upstream issue with the votable output (discovered thanks to this PR), most likely introduced in VizieR v7.33.3 . It is already solved in the beta version of VizieR, so the fix will be here automatically with the next release upstream.

ManonMarchand commented 5 months ago

Strange. Is there no action running the remote tests? I know that the UCD test fails.

bsipocz commented 5 months ago

Strange. Is there no action running the remote tests? I know that the UCD test fails.

Nope, we run those once a week (and the job is always failing). In practice I run the relevant remotes before merging a PR to spot any new failures in the affected modules.

ManonMarchand commented 5 months ago

Sure, I'll do these modifications next week, thanks for the review :slightly_smiling_face: (I did not try without ELLIPSIS, that's my fault, there is no bug)

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.

Project coverage is 67.17%. Comparing base (462c2da) to head (0ffd5d4). Report is 66 commits behind head on main.

:exclamation: Current head 0ffd5d4 differs from pull request most recent head 9e85f4e

Please upload reports for the commit 9e85f4e to get more accurate results.

Files	Patch %	Lines
astroquery/vizier/core.py	81.81%	2 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #3028 +/- ## ========================================== - Coverage 67.35% 67.17% -0.18% ========================================== Files 236 231 -5 Lines 18320 18247 -73 ========================================== - Hits 12339 12258 -81 - Misses 5981 5989 +8 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ManonMarchand commented 5 months ago

Should be all good? I did not switch to public API in the non-remote tests since some utility methods are also tested and I don't have the time right now to look at it, but it's on my radar for a future PR

bsipocz commented 5 months ago

I did not switch to public API in the non-remote tests

That's more like a future looking wishlist item at least for the public API stuff as you said for some functionality it doesn't even make sense.

bsipocz commented 5 months ago

Thanks @ManonMarchand!

astropy / astroquery