DALResults.fieldname_with_ucd way too permissive

[msdemlei]

Consider the following code:

import pyvo

svc = pyvo.dal.SCSService("https://dc.g-vo.org/ppmxl/q/cone/scs.xml")
resultset = svc.search((10, 10), radius=0.01)
print(resultset.fieldname_with_ucd("pos.pm;pos.eq.ra"))

This currently prints e_raepRA, which is highly bizarre (it's the error in RA, nb not the error in pmra or something like that).

Looking at the implementation, this is not surprising: What the method picks is the first column with a UCD that shares an atom with the UCD passed in. Git blame tells me that this implementation was introduced in 2018 in commit ce037f28, with the (for this purpose) unhelpful commit message: "use pytest and mock rather than legacy unittest and background server".

The previous implemenation,

    try:
        iterchain = (
            self.getdesc(fieldname) for fieldname in self.fieldnames)
        iterchain = (field for field in iterchain if field.ucd == ucd)
        return next(iterchain).name
    except StopIteration:
        return None

seemed sensible (modulo the somewhat complicated formulation), so I wonder why Stefan would have made things so permissive, and that while fiddling with the test system.

And I have a hard time believing I'm the first to bite this. Anyway: does anyone see a strong reason not to go back to what this did before, which simplifies to:

for field in self.fielddescs:
  if field.ucd == ucd:
    return field.name

In case this is really too stringent in that we may want to discard a meta.main (or something like that) occasionally, we can perhaps add extra modes. But I'm really sure we should not be too clever here by default; that's a recipe for WTF moments...