ENH: registry to find SIA v2 services

[bsipocz]

So, this does the bare minimum, e.g. making it possible to search for SIA v2 services (the tricky part was to figure out the case sensitivity for the spelling of the service type, anyway).

The missing part from this PR is to make servicetype='image' to return both SIA and SIA2 services. As currently there is a one-on-one mapping in SERVICE_TYPE_MAP, I'm waiting for input if you have suggestions on how to resolve this without going brute force and start looping through them if there are multiple values provided.

In [1]: import pyvo

In [2]: image_services_v2 = pyvo.regsearch(servicetype='sia2')

In [3]: image_services_v1 = pyvo.regsearch(servicetype='sia')

In [4]: len([s for s in image_services_v1 if 'irsa' in s.ivoid])
Out[4]: 33

In [5]: len([s for s in image_services_v2 if 'irsa' in s.ivoid])
Out[5]: 58

Closes #397

[msdemlei]

As a minimal implementation, that's fine. However, with the SIA2 standard id, we'll have to be a bit more careful, as what we really need to test against is LIKE 'ivo://ivoa.net/std/sia#query-2.%', since we'll want to cover later minor versions, too. At this point, this doesn't matter, and it'll certainly be years until that becomes an issue (no real plans for SIA 2.1 at this point). I think I'll tackle the whole image question before the next release, and I'll be happy to implant the minor version resilience at that point.

[bsipocz]

I'll open an issue about the minor versioning. Also, I'm not sure we fully cover all cases, as e.g we use # as a separator for a few places in the code, therefore could potentially miss out the distinction between sia and sia2. However, during testing I didn't really trigger those parts of the code.

Another potential issue could be the case sensitivity in the code vs the standard id (ivo://ivoa.net/std/sia#query-2... vs ivo://ivoa.net/std/SIA#query-2...

Anyway, I think we are not introducing any new bugs with this PR but fix some previous problems, so I would like to go ahead and merge and fix the remaining things in follow-ups.

[msdemlei]

On Tue, Feb 21, 2023 at 11:13:14AM -0800, Brigitta Sipőcz wrote: I'll open an issue about the minor versioning. Also, I'm not sure we fully cover all cases, as e.g we use `#` as a separator for a few places in the code, therefore could potentially miss out the distinction between sia and sia2. However, during testing I didn't really trigger those parts of the code.

I'm pretty sure the hash is never used as a separator where these standardIDs might turn up. Of course, there's always the possibility of... let's say a form of myopia, but I'd be rather relaxed about this part.

Another potential issue could be the case sensitivity in the code vs the standard id (`ivo://ivoa.net/std/sia#query-2...` vs `ivo://ivoa.net/std/SIA#query-2...`

This is covered by RegTAP, which guarantees that all these identifiers are lowercased on ingestion into the database; hence, on the registry side, we only see them lowercased. Case-insensitivity is still evil, and *if* we ever fetch these identifiers from, say, capability documents, we will have to do some sort of case folding.

Anyway, I think we are not introducing any new bugs with this PR but fix some previous problems, so I would like to go ahead and merge and fix the remaining things in follow-ups.

Sure. Thanks!

[codecov]

Codecov Report

Merging #422 (4f9dfce) into main (03d5541) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #422   +/-   ##
=======================================
  Coverage   79.81%   79.81%           
=======================================
  Files          52       52           
  Lines        5989     5989           
=======================================
  Hits         4780     4780           
  Misses       1209     1209           

Impacted Files	Coverage Δ
pyvo/registry/rtcons.py	97.67% <ø> (ø)
pyvo/registry/regtap.py	79.03% <100.00%> (ø)

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

[bsipocz]

On Tue, Feb 21, 2023 at 11:13:14AM -0800, Brigitta Sipőcz wrote:
I'll open an issue about the minor versioning. Also, I'm not sure
we fully cover all cases, as e.g we use # as a separator for a
few places in the code, therefore could potentially miss out the
distinction between sia and sia2. However, during testing I didn't
really trigger those parts of the code.

I'm pretty sure the hash is never used as a separator where these
standardIDs might turn up. Of course, there's always the possibility
of... let's say a form of myopia, but I'd be rather relaxed about
this part.

I suppose these were lines where I wondered whether it would do what we want for SIA2, where the hash is part of the standardid, rather than to indicate the version of the same service, but then the line was never really used during the tests, so I didn't dive into more details.

https://github.com/astropy/pyvo/blob/main/pyvo/registry/regtap.py#L311
https://github.com/astropy/pyvo/blob/main/pyvo/registry/regtap.py#L338

[bsipocz]

Tests are now all happy, so I go ahead and merge and open follow-up issues to track the potential problems we identified.

[msdemlei]

On Tue, Feb 21, 2023 at 12:25:02PM -0800, Brigitta Sipőcz wrote: I suppose these were lines where I wondered whether it would do what we want for SIA2, where the hash is part of the standardid, rather than to indicate the version of the same service, but then the line was never really used during the tests, so I didn't dive into more details. https://github.com/astropy/pyvo/blob/main/pyvo/registry/regtap.py#L311 https://github.com/astropy/pyvo/blob/main/pyvo/registry/regtap.py#L338

Oh dear... I was overconfident again. The code (which is there to support auxiliary capabilities as per https://ivoa.net/documents/discovercollections/20190520/index.html) assumes that incompatible standards have different StandardsRegExt records and hence different ivoids, which they *really* should. I clearly have not paid attention to that for SIAv2, and neither has anyone else. So, SIAv1 and SIAv2 use the same (basic) standard id. But the fragments aren't supposed to distinguish major versions. Ouch. Well, that milk is spilled; there's no way we can fix the identifiers now. I'll try to come up with some workaround.

[bsipocz]

Well, that milk is spilled; there's no way we can fix the identifiers
now. I'll try to come up with some workaround.

As long as the SIA cases are the only one, we can simply add a conditional for them in the code. Anyway, I opened #425 as getting more test coverage may likely smoke out edge cases like these.