Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: cadc invompatibility with dev versions of numpy #2811

Closed
bsipocz opened this issue Aug 15, 2023 · 7 comments · Fixed by #2814
Closed

BUG: cadc invompatibility with dev versions of numpy #2811

bsipocz opened this issue Aug 15, 2023 · 7 comments · Fixed by #2814

Comments

@bsipocz
Copy link
Member

bsipocz commented Aug 15, 2023

It's not yet clear whether it's an astroquery issue or something upstream, so I'm cc-ing @pllim, too.
It showed up 2 days ago, CI runs 3 days ago were still fine.

=================================== FAILURES ===================================
________________________________ test_exec_sync ________________________________

tmp_path = PosixPath('/tmp/pytest-of-runner/pytest-0/test_exec_sync0')

    @patch('astroquery.cadc.core.get_access_url',
           Mock(side_effect=lambda x, capability=None: 'https://some.url/'))
    def test_exec_sync(tmp_path):
        # save results in a file
        # create the VOTable result
        # example from http://docs.astropy.org/en/stable/io/votable/
        votable = VOTableFile()
        resource = Resource()
        votable.resources.append(resource)
        table = Table(votable)
        resource.tables.append(table)
        table.fields.extend([
            Field(votable, name="filename", datatype="char", arraysize="*"),
            Field(votable, name="matrix", datatype="double", arraysize="2x2")])
        table.create_arrays(2)
        table.array[0] = ('test1.xml', [[1, 0], [0, 1]])
        table.array[1] = ('test2.xml', [[0.5, 0.3], [0.2, 0.1]])
        buffer = BytesIO()
        votable.to_xml(buffer)
        cadc = Cadc(auth_session=requests.Session())
        response = Mock()
        response.to_table.return_value = table.to_table()
        cadc.cadctap.search = Mock(return_value=response)
    
        output_files = [os.path.join(tmp_path, 'test_vooutput.xml'),
                        Path(tmp_path, 'test_path_vooutput.xml')]
    
        for output_file in output_files:
            cadc.exec_sync('some query', output_file=output_file)
    
            actual = parse(output_file)
            assert len(votable.resources) == len(actual.resources) == 1
            assert len(votable.resources[0].tables) ==\
                len(actual.resources[0].tables) == 1
            actual_table = actual.resources[0].tables[0]
    
>           assert report_diff_values(table, actual_table, fileobj=sys.stdout)
E           assert False
E            +  where False = report_diff_values(<VOTable length=2>\n filename    matrix   \n  object  float64[2,2]\n--------- ------------\ntest1.xml   1.0 .. 1.0\ntest2.xml   0.5 .. 0.1, <VOTable length=2>\n filename    matrix   \n  object  float64[2,2]\n--------- ------------\ntest1.xml     -- .. --\ntest2.xml     -- .. --, fileobj=<_io.TextIOWrapper name="<_io.FileIO name=6 mode='rb+' closefd=True>" mode='r+' encoding='utf-8'>)
E            +    where <_io.TextIOWrapper name="<_io.FileIO name=6 mode='rb+' closefd=True>" mode='r+' encoding='utf-8'> = sys.stdout

../../.tox/py311-test-alldeps-devdeps-cov/lib/python3.11/site-packages/astroquery/cadc/tests/test_cadctap.py:397: AssertionError
----------------------------- Captured stdout call -----------------------------
  a>  filename   matrix  
   ?            -       -
  b>  filename  matrix 
  a> --------- ----------
   ?                   --
  b> --------- --------
  a> test1.xml 1.0 .. 1.0
  a> test2.xml 0.5 .. 0.1
  b> test1.xml -- .. --
  b> test2.xml -- .. --

---------- coverage: platform linux, python 3.11.4-final-0 -----------
@pllim
Copy link
Member

pllim commented Aug 15, 2023

Huh, so the tables are masked now? Weird...

There are not changes directly to astropy.io.votable nor astropy.utils.xml in the past few days that would affect functionality. I did merge this 3 days ago and there was a fix to astropy.units. Do you think it is because of that?

@bsipocz
Copy link
Member Author

bsipocz commented Aug 15, 2023

Hmm, locally I'm not yet able to reproduce, tried a few numpy versions as well as astropy, but haven't dived into a proper bisect on them. Could it be an OSX/Linux difference?
(I recall seeing some similar traceback in the failing astropy tests before the legacy printing was fixed.)

@pllim
Copy link
Member

pllim commented Aug 15, 2023

I wonder if report_diff_values broke. Not sure how well this function is tested upstream. The module itself has not been touched for 9 months but it appears you use it for comparison.

https://github.com/astropy/astropy/blob/main/astropy/utils/diff.py

@pllim
Copy link
Member

pllim commented Aug 15, 2023

Lots of numpy type checks in there... but do you really need this to compare tables?

@bsipocz
Copy link
Member Author

bsipocz commented Aug 15, 2023

but do you really need this to compare tables

Maybe not, but there is no reason it should stop working. And most annoyingly it still works-for-me ™️

I'll try to see what CI does if I switch out to OSX

@pllim
Copy link
Member

pllim commented Aug 15, 2023

Yes, definitely a concern if the function broke silently upstream. If you are able to narrow down the problem, please let me know!

I vaguely remember astropy.table.Table had problem with stdout being non-deterministic, but this is votable and the other problem was either due to win32 or the screen size not set in CI, which seem unrelated to your setup here?

@bsipocz bsipocz changed the title BUG: cadc invompatibility with dev versions of astropy/numpy BUG: cadc invompatibility with dev versions of ~astropy~/numpy Aug 16, 2023
@bsipocz bsipocz changed the title BUG: cadc invompatibility with dev versions of ~astropy~/numpy BUG: cadc invompatibility with dev versions of numpy Aug 16, 2023
@bsipocz
Copy link
Member Author

bsipocz commented Aug 16, 2023

I finally started to see this locally, too and it comes down to numpy-dev. I have the error using the latest nightly wheel, but I'll try to bisect as soon as possible using local builds, but I keep running into the meson issues, even when not building with spin, so the whole process may take longer than ideal.

Either case, I'm cc-ing @seberg in case he spots something trivial (or recall a recent PR that could cause it, I looked at the list but didn't see something obvious).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants