Add a check for FITS files mistaken as ASCII to astropy_table_read for Python 3.11 compatibility

[dhomeier]

Description

Trying now that Astropy is testing with 3.11.0rc2

[dhomeier]

Failure in data_factories.astropy_tabular_data('single_table.fits') – see astropy/astropy#13670.
Maybe the Table.read(*args, format='ascii', **kwargs) attempt can be skipped now as the automatic format detection should long have been fixed.

[dhomeier]

Maybe the Table.read(*args, format='ascii', **kwargs) attempt can be skipped now as the automatic format detection should long have been fixed.

Somewhat unexpectedly just trying Table.read(guess=True) is still not detecting those ASCII formats widely enough, so I have put the extra check inside the existing wrapper. Also not so surprising, there is some resistance to have such a check basically override the specified format='ascii' within astropy.table, so this may need to stay in here.

[dhomeier]

To sum up some of the discussion in astropy/astropy#13670 (comment), a somewhat cleaner solution would indeed be to first try the read with automatic format recognition (which would correctly identify the FITS file) and then in case of failure make a second attempt with format='ascii'.
That approach would also work, but change some results from the present behaviour, e.g. the test csv data
"#a,b\n1,2" is parsed as standard header (interpreting the column names as ['#a', 'b']) when using full automatic detection, but as ascii.commented_header (column names ['a', 'b']) when reading with format='ascii'.
So the tests would need to be adapted, but more importantly the default behaviour would change from what some users might have become used to.

[dhomeier]

Matplotib 3.6.x errorbar failures seem to be indeed specific to Python 3.10; passing at least with 3.11rc2.

[astrofrog]

@dhomeier - could you rebase this? The errorbar error should be gone now.

[dhomeier]

Yes, and the remaining tests should mostly be trimmed back to those from #2334 – I'd probably add py311-pyqt64 and py311-pyside64, (or :63, depending on the status of #2318).

[dhomeier]

macos: py311 probably needs brew hdf5 as well.

[dhomeier]

I suspect macos: py311 yet needs something like HDF5_DIR=/usr/local/homebrew/opt/hdf5 (or wherever its default install path is, but why is homebrew not setting this up properly on its own?!?) added to $GITHUB_ENV, but suppose this is still only a feature under discussion in OpenAstronomy/github-actions-workflows#56.

[astrofrog]

Since the Python 3.11 release is imminent, maybe we can wait a few days for this to see if we can at least avoid having to hard-code the Python version to rc2?

I think we'll probably just have to wait for h5py to have Python 3.11 wheels for things to work correctly on MacOS X, but since that isn't really a problem on our side we could just comment out the MacOS X Python 3.11 testing for now?

[dhomeier]

Yes, having a default version to use in ci_workflows would be good.
The tests can actually run without h5py, but to properly set this up, I should perhaps figure out how to have h5py as a test-dep only for OS != macOS platform_system=='Linux' – I seem to have missed that it fails to build on Windows as well! 😄

[dhomeier]

No 3.11.0 for macOS yet. The Windows failures might rather be pandas errors. And this

File "/home/runner/work/glue/glue/.tox/py38-test-pyqt514-all/lib/python3.8/site-packages/pytestqt/logging.py", line 5, in
from py._code.code import TerminalRepr, ReprFileLocation
ModuleNotFoundError: No module named 'py._code'; 'py' is not a package

seems just randomly broken today!

[pllim]

new pytest was released too 😬

[dhomeier]

So that's the nose deprecation in Astropy? Had been wondering if they'd deprecate that between 7.1 and 7.2.

[dhomeier]

But generally, if h5py is removed from install_requires here and just kept as test dep, that will not affect your test-deps downstream, right?

[pllim]

if h5py is removed from install_requires here and just kept as test dep, that will not affect your test-deps downstream, right?

Correct.

[dhomeier]

So the py dependency was actually missing from pytest-qt==4.1.0, which is strangely pulled in by some runs, but not others. Was just removed in pytest-qt 4.2.0, so hopefully that should sort itself out.

[dhomeier]

@pllim would this be acceptable for resolution of the h5py dependency issue in lieu of #2341?

[pllim]

Why do you list h5py multiple times? Is the spec in install_requires not enough?

[dhomeier]

This would still install it in 3.11 with glueviz[all] (I hope); the platform_system=="Linux" in test below is added as an OR so we are still running the full tests with linux-py311; the second python_version<'3.11' there is redundant, admittedly.

[pllim]

Yes, if you deem it sufficient, you can close #2341 without merge and go with this PR. Thanks!

[astrofrog]

This looks good to me, feel free to merge once you get the CI to pass

[dhomeier]

macOS with 3.11 now works (without h5py); pyside63 seems a bit unstable...