fix to registry.regsearch() waveband option

[trjaffe]

I am not sure why this partly worked before, but it did not find matches to entries with multiple wavebands like ("optical#infrared#uv"). Try

uv_services=vo.regsearch(servicetype='image',keywords='galex',waveband='uv')
print(uv_services.to_table()['waveband','short_name'])

and see the extra stuff picked up after I think I fixed the ivo_hashlist_has() call, which I think was backward compared to documentation in http://www.ivoa.net/documents/RegTAP/20141208/REC-RegTAP-1.0.html .

[cbanek]

Okay, so investigating this a bit further.

I see the more results, and that makes me feel like you're definitely doing the right thing with this PR code wise. I was confused about how we could have gotten the order of parameters wrong, so I was looking in the docs a bit more...

In the link of RegTAP that you give above, that is the 1.0 released version, and in section 10.3 it has a good example that does exactly this thing, and it looks like:

SELECT ivoid, access_url 
FROM rr.capability 
  NATURAL JOIN rr.resource
  NATURAL JOIN rr.interface
WHERE standard_id='ivo://ivoa.net/std/sia'
  AND intf_type='vs:paramhttp'
  AND 1=ivo_hashlist_has('infrared', waveband)

Now, this gives weight to the previous way of doing it, as infrared is first, then waveband.

But in the latest draft (http://www.ivoa.net/documents/RegTAP/20190911/PR-RegTAP-1.1-20190911.html#tth_sEc10.3) the order of parameters of the same example is backward:

SELECT ivoid, access_url
FROM rr.capability 
  NATURAL JOIN rr.resource
  NATURAL JOIN rr.interface
WHERE standard_id LIKE 'ivo://ivoa.net/std/sia%'
  AND intf_role='std'
  AND 1=ivo_hashlist_has(waveband, 'infrared')

Now, looking at the errata and changes, I don't see any mentions of this changing, although it obviously did.

Looking at all the drafts to see where the change happened, I have found that this one (http://ivoa.net/documents/RegTAP/20180731/PR-RegTAP-1.1-20180731.html#tth_sEc10.3) still has it the reverse, and the next version (http://ivoa.net/documents/RegTAP/20190326/PR-RegTAP-1.1-20190326.html#tth_sEc10.3) has corrected the order to what you've done.

So I think how we got this was that we were following the examples, but the example was actually wrong. In the rest of the document, talking about the format of ivo_hashlist_has it always has stayed the same, first the list (with the hashes in between items) and then the one you're searching for. It looks like the example was just backward.

So all that being said, I think you're doing the right thing and I approve. I'll give it some time if @funbaker or maybe @msdemlei want to chime in with any further thoughts, but I'd like to take this for the 1.0 release.

GREAT FIND. Thank you so much. This is a really tricky and subtle bug.

[msdemlei]

Hi all,

On Tue, Sep 17, 2019 at 02:55:26PM -0700, Christine Banek wrote: Now, this gives weight to the previous way of doing it, as infrared is first, then waveband. But in the latest draft (http://www.ivoa.net/documents/RegTAP/20190911/PR-RegTAP-1.1-20190911.html#tth_sEc10.3) the order of parameters of the same example is backward:

Yes, my bad.

``` SELECT ivoid, access_url FROM rr.capability NATURAL JOIN rr.resource NATURAL JOIN rr.interface WHERE standard_id LIKE 'ivo://ivoa.net/std/sia%' AND intf_role='std' AND 1=ivo_hashlist_has(waveband, 'infrared') ``` Now, looking at the errata and changes, I don't see any mentions of this changing, although it obviously did.

Well, it's a bit obscure in the current PR; the last bullet point has "repaired a bad argument order in 10.3". I give you you can't be expected to find this. I've changed that to "and repaired the bad order of arguments in ivo\_hashlist\has in query 10.3". I'd hope that's simpler to find.

So all that being said, I think you're doing the right thing and I approve. I'll give it some time if @funbaker or maybe @msdemlei want to chime in with any further thoughts, but I'd like to take this for the 1.0 release.

Yes, sure. Merge it. One thing I'm wondering, though: Should we support lists of wavebands here? ["infrared", "optical"] would seem a fairly common thing people might want, at least until http://ivoa.net/documents/Notes/Regstc/ has made it. If you agree, should and I'll prepare a patch.

[cbanek]

Sounds good, thanks everyone!

As to the question of should we support multiple waveband searching, I think that sounds like a fantastic idea, created #176 . Although since that's a new feature, I don't think I'll hold up the release for that one.