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.

Sign up for GitHub

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

Jump to bottom

Drop support for Python 3.7 #210

Merged
merged 4 commits into from
Feb 7, 2023
Merged

Drop support for Python 3.7 #210

merged 4 commits into from
Feb 7, 2023

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Feb 6, 2023 •
edited
Loading

In addition:

  • Make annotations compatible with oldest supported Python version (mypy was failing on Python 3.8)
  • Test pre-commit in all supported versions
  • Updated isort dependency

Closes #76

@ricardoV94 ricardoV94 requested a review from ferrine February 6, 2023 15:12
@ricardoV94 ricardoV94 changed the title Make annotations compatible with oldest supported Python version Drop support for Python 3.7 Feb 6, 2023
@ricardoV94 ricardoV94 requested a review from maresb February 6, 2023 15:13
@ricardoV94
Copy link
Member Author

Codecov upload failed... dunno if it's a fluke

ferrine
ferrine reviewed Feb 6, 2023
View reviewed changes
pytensor/link/numba/dispatch/elemwise_codegen.py
@@ -1,3 +1,5 @@
from __future__ import annotations
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Otherwise in Python 3.8 you get pytensor/link/numba/dispatch/elemwise_codegen.py:72: error: "list" is not subscriptable, use "typing.List" instead

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm okay either way, but that's why I changed it to run pre-commit in all supported versions. We missed this issue because it was running only in 3.9

@Armavica
Copy link
Member

Armavica commented Feb 6, 2023

Should we maybe add pyupgrade to pre-commit, like for PyMC?

@ricardoV94
Copy link
Member Author

Should we maybe add pyupgrade to pre-commit, like for PyMC?

Yup. Mind opening an issue / PR, or do you think it should be done here?

@Armavica
Copy link
Member

Armavica commented Feb 6, 2023 •
edited
Loading

Yeah no worries, I can create a separate PR, once this one is merged.

maresb
maresb reviewed Feb 7, 2023
View reviewed changes
.github/workflows/test.yml
@@ -50,11 +50,14 @@ jobs:
needs: changes
runs-on: ubuntu-latest
if: ${{ needs.changes.outputs.changes == 'true' }}
strategy:
matrix:
python-version: ["3.8", "3.9", "3.10"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Conda-forge builds for 3.11 so we should probably be testing it upstream here.

Suggested change
python-version: ["3.8", "3.9", "3.10"]
python-version: ["3.8", "3.9", "3.10", "3.11"]

Would it make sense (for energy/environment) to drop tests for 3.9 and 3.10 since they're in the middle, and most failures will occur in the extreme versions?

Copy link
Member

Choose a reason for hiding this comment

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

AFAIK numba is not yet available on 3.11 but that should not stop us.
Looking forward to all these speed ups

Copy link
Member Author

@ricardoV94 ricardoV94 Feb 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

There's another PR for 3.11: #198.

We could test support for it but would need to tweak jobs so that they run all tests (at least once), except for the numba ones.

Pushes there are welcome

pyproject.toml
@@ -34,7 +34,6 @@ classifiers = [
"Operating System :: Unix",
"Operating System :: MacOS",
"Programming Language :: Python :: 3",
"Programming Language :: Python :: 3.7",
"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why the diff looks weird, I'm just adding a line for 3.11:

Suggested change
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",

maresb
maresb approved these changes Feb 7, 2023
View reviewed changes
Copy link
Contributor

@maresb maresb left a comment

Choose a reason for hiding this comment

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

In light of #210 (comment), LGTM

@ricardoV94 ricardoV94 merged commit 35f3cbf into pymc-devs:main Feb 7, 2023
@ricardoV94 ricardoV94 deleted the annotations_compat branch February 7, 2023 10:27
@ricardoV94 ricardoV94 mentioned this pull request Feb 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ferrine ferrine ferrine left review comments

@michaelosthege michaelosthege michaelosthege approved these changes

@maresb maresb maresb approved these changes

@twiecki twiecki twiecki approved these changes

Assignees
No one assigned
Labels
GitHub CI/CD maintenance major release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Seems like Numpy has dropped python 3.7 in v1.23.5
6 participants
@ricardoV94 @Armavica @twiecki @michaelosthege @ferrine @maresb