vo_conesearch errors with astropy-dev

[bsipocz]

There are quite a few errors coming for astroquery.vo_conesearch when testing with the current dev version of astropy

See: https://github.com/astropy/astroquery/pull/2050/checks?check_run_id=2500552000

(and also there are a ton of warnings, but maybe we can ignore those...)

[bsipocz]

cc @pllim

[pllim]

Yeah, @tomdonaldson and I already fantasize about retiring vo_conesearch and use something more modern over at PyVO ecosystem, so it is fine to ignore all the warnings from vo_conesearch. I apologize for any inconvenience caused!

[pllim]

Oh, there are errors too, eh? Hmm, I'll have to investigate, but still, you can ignore the warnings.

[bsipocz]

Yes, I'm ignoring all vo warnings, some are cone search related some are coming from various votable parsings in other modules.

However the error is unrelated to the cleanup and scrictening warning handling, as they show up also in Adam's pr.

[bsipocz]

But if vo cone search is going away in the short timescale then I'm perfectly happy with ignoring the errors with an xfail for now.

[pllim]

I wouldn't hold my breath. I will see if I can fix the errors. Thanks for bringing this to my attention!

[pllim]

p.p.s. I am really confused now. I cannot reproduce the error locally. Investigation continues.

[pllim]

p.p.p.s. Why is JSON trying to decode what looks like a FITS header? 🤯

[pllim]

The traceback from CI makes no sense. Why am I seeing a FITS header here? It is supposed to be reading a JSON file off test data dir. I tried raising errors with filename and content, but when I did that, it gave me the stuff I expected.

2021-05-05T19:39:59.7014137Z ____________ ERROR at setup of TestConeSearchResults.test_listcats _____________
2021-05-05T19:39:59.7014886Z 
2021-05-05T19:39:59.7016492Z self = <class 'astroquery.vo_conesearch.validator.tests.test_inpect.TestConeSearchResults'>
2021-05-05T19:39:59.7017266Z 
2021-05-05T19:39:59.7017597Z     def setup_class(self):
2021-05-05T19:39:59.7018161Z         self.datadir = 'data'
2021-05-05T19:39:59.7018683Z         test_vos_path = get_pkg_data_path(self.datadir) + os.sep
2021-05-05T19:39:59.7019295Z     
2021-05-05T19:39:59.7020143Z         # Convert to a proper file:// URL--on *NIXen this is not necessary but
2021-05-05T19:39:59.7021041Z         # Windows paths will blow up if we don't do this.
2021-05-05T19:39:59.7021828Z         test_vos_path = '/'.join(test_vos_path.split(os.sep))
2021-05-05T19:39:59.7022536Z         if not test_vos_path.startswith('/'):
2021-05-05T19:39:59.7023200Z             test_vos_path = '/' + test_vos_path
2021-05-05T19:39:59.7023572Z     
2021-05-05T19:39:59.7024170Z         cs_conf.vos_baseurl = 'file://' + test_vos_path
2021-05-05T19:39:59.7024811Z >       self.r = inspect.ConeSearchResults()
2021-05-05T19:39:59.7025243Z 
2021-05-05T19:39:59.7025782Z ../../astroquery/vo_conesearch/validator/tests/test_inpect.py:39: 
2021-05-05T19:39:59.7026333Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
2021-05-05T19:39:59.7027074Z ../../astroquery/vo_conesearch/validator/inspect.py:48: in __init__
2021-05-05T19:39:59.7028090Z     self.dbs[typ] = get_remote_catalog_db(
2021-05-05T19:39:59.7029117Z ../../astroquery/vo_conesearch/vos_catalog.py:663: in get_remote_catalog_db
2021-05-05T19:39:59.7029787Z     return VOSDatabase.from_json(
2021-05-05T19:39:59.7030582Z ../../astroquery/vo_conesearch/vos_catalog.py:525: in from_json
2021-05-05T19:39:59.7031129Z     tree = json.load(fd)
2021-05-05T19:39:59.7031998Z /opt/hostedtoolcache/Python/3.9.4/x64/lib/python3.9/json/__init__.py:293: in load
2021-05-05T19:39:59.7032607Z     return loads(fp.read(),
2021-05-05T19:39:59.7033203Z /opt/hostedtoolcache/Python/3.9.4/x64/lib/python3.9/json/__init__.py:346: in loads
2021-05-05T19:39:59.7033877Z     return _default_decoder.decode(s)
2021-05-05T19:39:59.7034563Z /opt/hostedtoolcache/Python/3.9.4/x64/lib/python3.9/json/decoder.py:337: in decode
2021-05-05T19:39:59.7035257Z     obj, end = self.raw_decode(s, idx=_w(s, 0).end())
2021-05-05T19:39:59.7035728Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
2021-05-05T19:39:59.7035969Z 
2021-05-05T19:39:59.7036515Z self = <json.decoder.JSONDecoder object at 0x7f1131b67070>
2021-05-05T19:39:59.7037692Z s = 'SIMPLE  =                    T / conforms to FITS standard                      BITPIX  =                    8 / arra...                                                                                                                      '
2021-05-05T19:39:59.7038257Z idx = 0

[pllim]

OK I can reproduce it now but only by running python setup.py test, not when I run pytest directly. Something is modifying some sort of global state that somehow my test is sensitive to.

[pllim]

I wonder if this is naturally solved if you apply APE 17. I don't think it is a bug within vo_conesearch. If you want, the temporary solution can be to xfail it in CI.

[pllim]

Hmm... I wonder if this is caused by your monkeypatch to astropy/utils/data.py functions.

[bsipocz]

OK I can reproduce it now but only by running python setup.py test, not when I run pytest directly. Something is modifying some sort of global state that somehow my test is sensitive to.

Weird as we run pytest with tox in CI. Maybe it's a minor version bug somewhere, or as you say due to the utils.data changes.

[pllim]

Okay, I strongly suspect astropy/astropy#10434 . The fix is either patch your monkeypatch in astroquery/utils/common.py or stop monkeypatching altogether.

[pllim]

Yeah, narrowed it down to some weird interactions between tests in utils and vo_conesearch. You get the errors you are seeing if you run this:

python setup.py test -P utils,vo_conesearch

You get new set of errors if you run this:

python setup.py test -P vo_conesearch,utils

🤯

[bsipocz]

ouch, that -P should not be order sensitive, at least I would have assumed it shouldn't...

[bsipocz]

Given this only affects the test monkeypatches, would you think it's safe to assume no users would run into issues due to this? If so, then xfail and an issue for refactoring the monkeypatch could be a way to go.

[pllim]

I might have found a fix over at #2059 . Though we're going to have this problem again manifesting next time someone touches astropy/utils/data.py upstream. I would very much be in support of refactoring the monkeypatching here regardless.