BUG: SIA2 position search to be SkyCoord aware

[bsipocz]

To fix #305

However, this is very much the bare minimum, as it still doesn't handle the case of vector skycoord input, neither non circular search, so eventually a more thorough overhaul will be required.

[codecov]

Codecov Report

Merging #459 (6e50499) into main (b96a950) will decrease coverage by 0.02%.
The diff coverage is 72.72%.

@@            Coverage Diff             @@
##             main     #459      +/-   ##
==========================================
- Coverage   80.00%   79.98%   -0.02%     
==========================================
  Files          52       52              
  Lines        6027     6036       +9     
==========================================
+ Hits         4822     4828       +6     
- Misses       1205     1208       +3     

Files Changed	Coverage Δ
pyvo/dal/params.py	85.71% <72.72%> (-0.67%)	⬇️

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

[msdemlei]

I like it. While we're doing this: is there any concept of spherical circle and polygon in astropy that we ought to understand here, too? I'm also asking because it'd be nice to support these in Registry discovery.

[bsipocz]

I suppose we could use astropy regions https://astropy-regions.readthedocs.io/en/stable/index.html?

Cc'ing @keflavich and @andamian as astroquery.alma already uses it on the user api (but then I suppose it just wraps back to this under the hood, I'll check it when I'm back at the keyboard)

[keflavich]

I'm not sure I understood the question, but yes, I would advocate for using astropy regions anywhere we're talking about sky regions.

[bsipocz]

I'm not sure how to make the RANGE and POLYGON easily work with SkyCoord as well as to support vector SkyCoord queries. Maybe the logic could be: check whether the first element is a SkyCoord. Check whether the last one if an angle. If so, do a CIRCLE query, if not, then use the vector SkyCoord elements as points in the RANGE or POLYGON?

[andamian]

This looks good. I don't think it will interfere with the new features you've mentioned.

[tomdonaldson]

LGTM also.

[bsipocz]

OK, given the approvals, let's have this in as a bugfix, the more comprehensive solution (something along the line of #459 (comment)) will come in a follow-up enhancement