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

Remove python 3.8 support and add python 3.13 support #2007

Draft
wants to merge 18 commits into
base: dev
Choose a base branch
from

Conversation

stephprince
Copy link
Contributor

@stephprince stephprince commented Nov 25, 2024

Motivation

Fix #1988. Python 3.8 has been deprecated - we will remove support for 3.8 and add support for python 3.13.

TODO

  • update GitHub branch protections to switch to new workflow minimums before merging

Checklist

  • Did you update CHANGELOG.md with your changes?
  • Have you checked our Contributing document?
  • Have you ensured the PR clearly describes the problem and the solution?
  • Is your contribution compliant with our coding style? This can be checked running ruff check . && codespell from the source directory.
  • Have you checked to ensure that there aren't other open Pull Requests for the same change?
  • Have you included the relevant issue number using "Fix #XXX" notation where XXX is the issue number? By including "Fix #XXX" you allow GitHub to close issue #XXX when the PR is merged.

@stephprince
Copy link
Contributor Author

stephprince commented Nov 26, 2024

I believe the python-3.13 gallery tests are failing because requirements-doc.txt requires dandi, which requires nwbinspector, which currently has h5py<3.12.0 pinned. h5py 3.12 is the earliest release with pre-built packages for python 3.13. Until h5py is unpinned in the inspector, using python 3.13 with the current requirements will try to build the h5py wheel.

NeurodataWithoutBorders/nwbinspector#536 is in progress to remove the h5py restriction in the inspector. We can finish that PR and make another nwbinspector release, or use 3.12 for the gallery and dandi tests, or fix the h5py build.

The cause of the excessive warnings in the gallery-3.9-minimum tests is less clear to me…. it seems to be coming from one of the dependencies for the linkml-runtime, schema sheets, or oaklib packages, which were not previously being installed with python 3.8. Unpinning the packages does not solve the warnings when using python 3.9.

@rly
Copy link
Contributor

rly commented Nov 26, 2024

Let's make another nwbinspector release.

Interesting. It looks like we have not actually been testing against minimum requirements. See https://github.com/NeurodataWithoutBorders/pynwb/actions/runs/12024183637/job/33519287151

It installs the minimum requirements first and then upgrades them.

py38-minimum: install_deps> python -m pip install -U -r requirements-dev.txt -r requirements-min.txt
.pkg: install_requires> python -I -m pip install hatch-vcs hatchling
.pkg: _optional_hooks> python /opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/pyproject_api/_backend.py True hatchling.build
.pkg: get_requires_for_build_sdist> python /opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/pyproject_api/_backend.py True hatchling.build
.pkg: freeze> python -m pip freeze --all
.pkg: hatch-vcs==0.4.0,hatchling==1.26.3,packaging==24.2,pathspec==0.12.1,pip==24.3.1,pluggy==1.5.0,setuptools==75.3.0,setuptools-scm==8.1.0,tomli==2.1.0,trove-classifiers==2024.10.21.16,typing_extensions==4.12.2,wheel==0.45.1
.pkg: build_sdist> python /opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/pyproject_api/_backend.py True hatchling.build
py38-minimum: install_package_deps> python -m pip install -U 'h5py>=2.10' 'hdmf>=3.14.5' 'numpy>=1.18' 'pandas>=1.1.5' 'python-dateutil>=2.7.3'

There should not be a -U on this line

pynwb/tox.ini

Line 16 in a63387d

python -m pip install -U {opts} {packages}

You could also change tox.ini to this abstracted, easier to update, but potentially more confusing tox.ini format that we use in HDMF: https://github.com/hdmf-dev/hdmf/blob/dev/tox.ini

@stephprince
Copy link
Contributor Author

Oh interesting, I think that tox.ini format you linked looks nice and easier to maintain. I will try updating to use that instead.

@stephprince
Copy link
Contributor Author

The cause of the excessive warnings in the gallery-3.9-minimum tests is less clear to me…. it seems to be coming from one of the dependencies for the linkml-runtime, schema sheets, or oaklib packages, which were not previously being installed with python 3.8. Unpinning the packages does not solve the warnings when using python 3.9.

I think there is something weird going on with the resolver between dependencies for the linkml-runtime, schemasheets, and oaklib packages and dandi. If you install dandi after installing everything else in requirements-doc.txt it installs fine with python 3.9. Everything also installs fine in python 3.10. I'm not sure if it's worth debugging more or, for the purposes of the minimum gallery tests, if it's ok to set the lower python bound to 3.10 for those extra packages as we had it set previously.

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

Successfully merging this pull request may close these issues.

[Feature]: Python 3.8
2 participants