[Issue 504] add "detail=min" parameter

[chmmao]

This is an attempt to address the Issue #504 by adding an additional parameter "detail=min" to the tables function.
When running the following code:

from pyvo import registry
registry.search(registry.Spatial((10, 10)), registry.Freetext("egal")) 

The runtime improves from 2s down to 1.2s (on my machine), while the output size is unchanged.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.31%. Comparing base (a9ac860) to head (86e52de).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #634   +/-   ##
=======================================
  Coverage   82.31%   82.31%           
=======================================
  Files          72       72           
  Lines        7429     7429           
=======================================
  Hits         6115     6115           
  Misses       1314     1314           

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

[ManonMarchand]

Hi and thanks for looking into this.

Two remarks:

1. This should be an option.

These details are interesting in some occasions.

The registry sub-module could then use the minimal details option per default, as I don't see where this could affect the users (but maybe I'm missing something? @msdemlei ?)

2. What about services that don't support details=min ?

I tried calling this on one VOSI 1.0 service that does not support details=min (namely SIMBAD TAP service) and it looks like this does not raise an error, the maximum details is returned. But I'm not sure that this would be the expected behavior for every VOSI 1.0 service.

[msdemlei]

On Mon, Jan 06, 2025 at 03:55:07AM -0800, Manon Marchand wrote: 1. This should be an option. These details are interesting in some occasions.

Oh, of course they are, but they will be pulled in automatically when they are needed with details=min, so I don't think there is any need to expose details=min to users.

2. What about services that don't support details=min ? I tried calling this on one VOSI 1.0 service that does not support details=min (namely SIMBAD TAP service) and it looks like this does not raise an error, the maximum details is returned. But I'm not sure that this would be the expected behavior for every VOSI 1.0 service.

That is unproblematic (at least if we got everything right); VOSI 1.1 was written so we have automatic fallback, and pyVO has implemented that for a long time. Basically, where a service does not do details=min, all metadata is filled in with the first call. If it does, a stub is left for the columns data, and as clients require metadata, an extra request is done. The only reason I can see to make this switchable is when server implemenations of details=min are faulty (it *is* a bit more complicated, in particular with delimited table identifiers). But my take is that rather than offer workarounds, we had better fix the server implementations...

[ManonMarchand]

Then it looks to me like no API change is needed, so I'll add the 1.6.1 milestone so that the changelog entry can be added. Don't hesitate to call for help if you have issues making the CI tests pass, @chmmao

[chmmao]

Thanks for your responses. I am wondering what is happening in the dal/tests/testadhoc.py and registry/tests/test_regtap.py so that the CI-devtests failed.. Not quite sure what should I do there.
@ManonMarchand @msdemlei

[bsipocz]

The devtest failures (astropy deprecation related) are unrelated and fixed separately waiting for final review.

The other failures in the remote-data tests seem to be present on the main branch, too and maybe @ManonMarchand can have a look at them separately.

[bsipocz]

Thank you! I rephrased the changelog, but given the discussion on the PR this seems to be good to go now.

[bsipocz]

Thank you @chmmao. While this not really a bugfix but an enhancement, I think we can keep it in the bugfix release as the performance improvement seems to be non-negligible.