BUG: SIA1's getdataurl() to favour VOX:Image_AccessReference

[bsipocz]

A snippet that fails with IRSA's cloud_access SIA column. We concluded that this is a bug in pyvo, and therefore this PR is a fix for it.

@tomdonaldson - I went ahead and did this PR as we plan to roll back the deployment, and it was easier to do the bugfix while it was still failing. Following heasarc's example, we do roll back to populate the ucd for that field, so older pyvo versions will stay compatible.

Snippet (though not minimal example) to reproduce:

import numpy as np

# For downloading files
from astropy.utils.data import download_file

from astropy.coordinates import SkyCoord
from astropy.io import fits
from astropy.nddata import Cutout2D
import astropy.visualization as vis
from astropy.wcs import WCS
from astroquery.ipac.ned import Ned

import pyvo as vo
objects_in_paper = Ned.query_refcode('2016ApJ...817..109O')
allwise_image_services = vo.regsearch(servicetype='image', keywords=['allwise'])
allwise_image_service = allwise_image_services[0]
galaxies = objects_in_paper[np.array(objects_in_paper['Type']) == 'G']
ra = galaxies['RA'][0]
dec = galaxies['DEC'][0]
pos = SkyCoord(ra, dec, unit = 'deg')
allwise_image_table = allwise_image_service.search(pos=pos, size=0)
for allwise_image_record in allwise_image_table:
    if 'W1' in allwise_image_record.bandpass_id:
        break
allwise_image_record.getdataurl()

[codecov]

Codecov Report

Merging #445 (cad92ba) into main (a5075ff) will increase coverage by 0.01%.
Report is 3 commits behind head on main.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main     #445      +/-   ##
==========================================
+ Coverage   79.98%   80.00%   +0.01%     
==========================================
  Files          52       52              
  Lines        6036     6035       -1     
==========================================
  Hits         4828     4828              
+ Misses       1208     1207       -1     

Files Changed	Coverage Δ
pyvo/dal/sia.py	62.44% <50.00%> (+0.25%)	⬆️

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

[tomdonaldson]

+1 for the British spelling.

[tomdonaldson]

The change looks good. Ideally I do think there should be a test case for this.

[bsipocz]

The modified xml data file caused the tests to fail on main, while the regression is fixed by this PR, so I believe it should be enough as a test.

================================================================================================= FAILURES ==================================================================================================
________________________________________________________________________________________________ test_search ________________________________________________________________________________________________

    @pytest.mark.usefixtures('sia')
    @pytest.mark.usefixtures('register_mocks')
    @pytest.mark.filterwarnings("ignore::astropy.io.votable.exceptions.W06")
    def test_search():
        results = search('http://example.com/sia', pos=(288, 15))
        result = results[0]
    
>       _test_result(result)

pyvo/dal/tests/test_sia.py:53: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

result = ('This should not be the dataurl', 'http://example.com/querydata/image.fits', 'image/fits', 153280, 18885.9416782409, ...-07, 5.2e-07, masked_array(data=[0.000403806, 0.000406123],
             mask=[False, False],
       fill_value=1e+20))

    def _test_result(result):
        print(result)
>       assert result.getdataurl() == 'http://example.com/querydata/image.fits'
E       AssertionError: assert 'This should ...e the dataurl' == 'http://examp...ta/image.fits'
E         - http://example.com/querydata/image.fits
E         + This should not be the dataurl

pyvo/dal/tests/test_sia.py:41: AssertionError
________________________________________________________________________________________ TestSIAService.test_search _________________________________________________________________________________________

self = <pyvo.dal.tests.test_sia.TestSIAService object at 0x12797a7d0>

    @pytest.mark.usefixtures('sia')
    @pytest.mark.usefixtures('register_mocks')
    @pytest.mark.filterwarnings("ignore::astropy.io.votable.exceptions.W06")
    @pytest.mark.filterwarnings("ignore::astropy.io.votable.exceptions.W42")
    @pytest.mark.filterwarnings("ignore::astropy.io.votable.exceptions.W49")
    def test_search(self):
        service = SIAService('http://example.com/sia')
    
        results = service.search(pos=(288, 15))
        result = results[0]
    
>       _test_result(result)

pyvo/dal/tests/test_sia.py:68: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

result = ('This should not be the dataurl', 'http://example.com/querydata/image.fits', 'image/fits', 153280, 18885.9416782409, ...-07, 5.2e-07, masked_array(data=[0.000403806, 0.000406123],
             mask=[False, False],
       fill_value=1e+20))

    def _test_result(result):
        print(result)
>       assert result.getdataurl() == 'http://example.com/querydata/image.fits'
E       AssertionError: assert 'This should ...e the dataurl' == 'http://examp...ta/image.fits'
E         - http://example.com/querydata/image.fits
E         + This should not be the dataurl

pyvo/dal/tests/test_sia.py:41: AssertionError
========================================================================================== short test summary info ==========================================================================================
SKIPPED [35] ../other/pytest-remotedata/pytest_remotedata/plugin.py:85: need --remote-data option to run
SKIPPED [1] ../../../.pyenv/versions/3.10.8/lib/python3.10/site-packages/_pytest/doctest.py:455: all tests skipped by +SKIP option
XFAIL pyvo/io/vosi/tests/test_tables.py::TestTables::test_wrong_flag
FAILED pyvo/dal/tests/test_sia.py::test_search - AssertionError: assert 'This should ...e the dataurl' == 'http://examp...ta/image.fits'
FAILED pyvo/dal/tests/test_sia.py::TestSIAService::test_search - AssertionError: assert 'This should ...e the dataurl' == 'http://examp...ta/image.fits'
=========================================================================== 2 failed, 281 passed, 36 skipped, 1 xfailed in 1.11s ============================================================================

[bsipocz]

can I have a rereview on this, too, please?

[tomdonaldson]

Yes, this looks good to me now. Thanks!