-
Notifications
You must be signed in to change notification settings - Fork 30
Add support for Python 3.14 #224
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apologies for the late review, but this is great, thanks! tragically there's now a conflict in the changelog. once that's resolved and this gets a second review, this is good to go!
Thanks for the review @wgreenberg! Just merged main to resolve the conflict. Should be good now. As mentioned in the description, a new release afterwards would be much appreciated 🙏🏻 It would help move testing Python 3.14 along. |
pyproject.toml
Outdated
| # This should be kept in sync with the version of Python specified in poetry's | ||
| # dependencies above. | ||
| target-version = ['py39', 'py310', 'py311', 'py312', 'py313'] | ||
| target-version = ['py39', 'py310', 'py311', 'py312', 'py313'] # TODO add 'py314' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODOs should have an associated issue tracking the plan to complete them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure that's really necessary here. I only added the comment since the current black version doesn't support py314. The line will also become redundant once the build-system is updated to poetry v2 which supports project.requires-python.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are the sorts of details that would be great in an issue so that we remember to update this code when they become available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #232 and updated the comment here.
|
Thanks for the review @ohemorange. I left comments on each of your suggestions. Overall I'd suggest to leave the PR as is. However my main focus is to add support for Python 3.14 and not to insist on my coding style. If, after reading my comments, you still prefer that I change it, please to tell and I'm happy to update the PR. |
ohemorange
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However my main focus is to add support for Python 3.14 and not to insist on my coding style.
It is important to have a readable, maintainable codebase. Coding style is in fact a part of reviewing PRs.
pyproject.toml
Outdated
| # This should be kept in sync with the version of Python specified in poetry's | ||
| # dependencies above. | ||
| target-version = ['py39', 'py310', 'py311', 'py312', 'py313'] | ||
| target-version = ['py39', 'py310', 'py311', 'py312', 'py313'] # TODO add 'py314' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are the sorts of details that would be great in an issue so that we remember to update this code when they become available.
cdce8p
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However my main focus is to add support for Python 3.14 and not to insist on my coding style.
It is important to have a readable, maintainable codebase. Coding style is in fact a part of reviewing PRs.
Absolutely! The point I wanted to preference here is that the patch doesn't need to conform to my style, it needs to fit in with the project.
I've just pushed a commit with the changes you requested.
pyproject.toml
Outdated
| # This should be kept in sync with the version of Python specified in poetry's | ||
| # dependencies above. | ||
| target-version = ['py39', 'py310', 'py311', 'py312', 'py313'] | ||
| target-version = ['py39', 'py310', 'py311', 'py312', 'py313'] # TODO add 'py314' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #232 and updated the comment here.
ohemorange
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! lgtm!
|
Thanks everyone! Would it be possible to do a new release soon? Would be great to have a Python 3.14 compatible release ready for testing. |
|
We plan to do a new release soon! |
With PEP 649 annotation handling in Python 3.14 will change. The Python docs provide a good guide on how to access all annotations in 3.14 and beyond. I've updated the code in
json_util.pyto use the new functions.Furthermore
argparse.FileTypewill be deprecated. It's recommend to use the regularopencontextmanager instead.Lastly, I've added
3.14to the CI test matrix and the corresponding classifier.--
If accepted, it would be awesome if a new version could be released soon as this would unblock further testing with Python 3.14 in downstream projects.