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

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

Merged
merged 7 commits into from
Oct 28, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions .github/workflows/ci_workflows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,21 @@ jobs:
with:
coverage: codecov
display: true
# The Linux PyQt 5.15 installation requires apt-getting its xcb deps and headless X11 display
# Linux PyQt 5.15 and 6.x installations require apt-getting xcb and EGL deps
# and headless X11 display; as of Python 3.11 Scipy and h5py also need their own deps.
libraries: |
apt:
- '^libxcb.*-dev'
- libxkbcommon-x11-dev
- libegl1-mesa
- libopenblas-dev
- libhdf5-dev
brew:
- enchant

envs: |
# Standard tests
# Linux builds - test on all supported PyQt5 and PySide2 versions,
# Linux builds - test on all supported PyQt5/6 and PySide2/6 versions,
# and include all dependencies in some builds
- linux: py38-test-pyqt514-all
- linux: py38-test-pyside514
Expand All @@ -44,14 +47,15 @@ jobs:
- linux: py310-test-pyside63
- linux: py310-test-pyqt63-all
- linux: py310-test-pyqt64-all
- linux: py311-test-pyqt514

# Documentation build
- linux: py38-docs-pyqt514
coverage: false
- macos: py39-docs-pyqt515
coverage: false

# Test a few configurations on MacOS X
# Test a few configurations on macOS
- macos: py38-test-pyqt514-all
- macos: py310-test-pyqt515
- macos: py310-test-pyside63
Expand All @@ -63,25 +67,34 @@ jobs:

# Test against latest developer versions of some packages
- linux: py310-test-pyqt515-dev-all
- linux: py311-test-pyqt64-dev

allowed_failures:
needs: initial_checks
uses: OpenAstronomy/github-actions-workflows/.github/workflows/tox.yml@v1
with:
coverage: codecov
display: true
# Linux PyQt 5.15 and 6.3 installations require apt-getting xcb and EGL deps
libraries: |
apt:
- '^libxcb.*-dev'
- libxkbcommon-x11-dev
- libegl1-mesa
- libopenblas-dev
- libhdf5-dev
brew:
- enchant
- hdf5
envs: |
# PySide6 6.4 failures due to https://github.com/spyder-ide/qtpy/issues/373
# and https://github.com/matplotlib/matplotlib/issues/24155
# Python 3.11.0 not yet available on all platforms.
- linux: py310-test-pyside64
- windows: py310-test-pyside64
- linux: py311-test-pyside64
- macos: py311-test-pyqt515
- macos: py311-test-pyside64
- windows: py311-test-pyqt515

# Windows docs build
- windows: py310-docs-pyqt515
Expand All @@ -95,7 +108,7 @@ jobs:
uses: OpenAstronomy/github-actions-workflows/.github/workflows/publish_pure_python.yml@v1
with:
# Setup PyQt5 deps and headless X server as per pyvista/setup-headless-display-action@v1
libraries: '^libxcb.*-dev libxkbcommon-x11-dev libgl1-mesa-glx xvfb'
libraries: '^libxcb.*-dev libxkbcommon-x11-dev libgl1-mesa-glx libopenblas-dev libhdf5-dev xvfb'
test_extras: 'test,qt'
test_command: Xvfb :99 -screen 0 1024x768x24 > /dev/null 2>&1 & sleep 3; DISPLAY=:99.0 pytest --pyargs glue
secrets:
Expand Down
5 changes: 4 additions & 1 deletion glue/core/data_factories/astropy_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ def astropy_table_read(*args, **kwargs):
# also more generally, we should first try the ASCII readers.
if 'format' not in kwargs:
try:
return Table.read(*args, format='ascii', **kwargs)
t = Table.read(*args, format='ascii', **kwargs)
# Double-check for certain FITS files that may be read in as ASCII in Python 3.11
if not (len(t) == 1 and [c.value[0] for c in t.itercols()][:3] == ['SIMPLE', '=', 'T']):
return t
except Exception:
pass

Expand Down
5 changes: 4 additions & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ install_requires =
ipykernel>=4.0,!=5.0.0,!=5.1.0
qtconsole>=4.3
dill>=0.2
h5py>=2.10; python_version<'3.11'
xlrd>=1.2
openpyxl>=3.0
h5py>=2.10
mpl-scatter-density>=0.7
pvextractor>=0.2

Expand Down Expand Up @@ -74,6 +74,7 @@ all =
scikit-image
PyAVM
astrodendro
h5py>=2.10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

spectral-cube
# See https://github.com/python-pillow/Pillow/issues/4509
# for why we exclude pillow 7.1.0
Expand All @@ -97,6 +98,8 @@ test =
pytest-cov
pytest-faulthandler
pytest-flake8
h5py>=2.10; platform_system=="Linux"
h5py>=2.10; python_version<'3.11'
dhomeier marked this conversation as resolved.
Show resolved Hide resolved
objgraph

[options.package_data]
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tox]
envlist =
py{38,39,310}-{codestyle,test,docs}-{pyqt514,pyqt515,pyside514,pyside515,pyqt63,pyside63}-all-{dev,legacy}
py{38,39,310,311}-{codestyle,test,docs}-{pyqt514,pyqt515,pyside514,pyside515,pyqt63,pyside63}-all-{dev,legacy}
requires = pip >= 18.0
setuptools >= 30.3.0

Expand Down