astropy / pyvo

An Astropy affiliated package providing access to remote data and services of the Virtual Observatory (VO) using Python.
https://pyvo.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
75 stars 52 forks source link

BUG: column order should have no effect #456

Open bsipocz opened 1 year ago

bsipocz commented 1 year ago

While I was fixing https://github.com/astropy/pyvo/pull/445 I noticed that the code picked the first column that matched the criteria (e.g. it did matter where I added the extra info in the test data). However, the column order should have no effect, e.g. we should parse all and raise an exception if there are multiple columns matching or have a better hierarchy for what to look for.

I'm opening this issue to make sure we double check all service types for this type of hidden regression/assumption.

msdemlei commented 1 year ago

Interesting point. I'll mention that I don't think any IVOA standard actually explicitly says what a client ought to do in the presence of multiple identified things. Outside of ancient SIA, the thing designated for "unique" (in whatever sense) selection (so far; MIVOT might change that) is the utype, so I've made a quick census of where utype is mentioned in pyvo. The nontrivial occurrences:

That's about it.

So... if we're serious about failing for non-unique matches, dal.query.DALResults.field_name_with_utype would be the place to do it.

Me... well, I'm not opposed to a reasonable failure mode (warning?), because it's probably cheap and it helps improving services. However, in general I think this kind of validation is the job of, well, validators, and I'd like pyVO to keep defaulting to best-effort operation (i.e., if we can halfway confidently figure out what the service authors wanted us to do, we should do it).

Meaning: I'll kindly review a PR, but I'll not prepare one myself.