Add Python 3.13 Support #2080
Add Python 3.13 Support #2080
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❌ Your project status has failed because the head coverage (84.45%) is below the target coverage (85.00%). You can increase the head coverage or adjust the target coverage. ❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files@@ Coverage Diff @@
## master #2080 +/- ##
==========================================
- Coverage 91.33% 84.45% -6.89%
==========================================
Files 66 61 -5
Lines 16274 14310 -1964
==========================================
- Hits 14864 12085 -2779
- Misses 1410 2225 +815
Flags with carried forward coverage won't be shown. Click here to find out more. see 20 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| @@ -46,7 +46,7 @@ setup( | |||
| extras_require={ | |||
| 'grpc': [ | |||
| 'grpcio>=1.59.0,<2.0', | |||
| 'protobuf>=4.21.6,<5.0' | |||
| 'protobuf>=4.21.6' | |||
There was a problem hiding this comment.
This seems like it needs to be updated to 5.27.2. System Tests are randomly failing on imports when we install old protobuf versions. I'm puzzled how the system tests managed to pass in my original PR.
| build_test: pytest | ||
| build_test: coverage | ||
| build_test: mako | ||
| build_test: hacking | ||
| build_test: pep8-naming | ||
| codegen: mako | ||
| codegen: packaging | ||
| codegen: grpcio-tools == 1.59.0 # First version to support Python 3.12 | ||
| codegen: grpcio-tools == 1.67.0 # First version to support Python 3.13 |
There was a problem hiding this comment.
Using grpcio-tools 1.67.0 all the time breaks compatibility with grpcio <= 1.67.0 and will likely force users to update protobuf.
The technique we used for nidaqmx-python is to generate code using the oldest compatible Python and oldest compatible grpcio-tools, check the generated code into version control, then test against newer Python and grpcio versions.
- Python 3.8 (for now): https://github.com/ni/nidaqmx-python/blob/master/.github/workflows/build.yml
- grpcio-tools 1.49.1 (as long as you are doing codegen with Python 3.8, 3.9, 3.10, or 3.11): https://github.com/ni/nidaqmx-python/blob/master/pyproject.toml#L65
There was a problem hiding this comment.
oldest compatible grpcio-tools
clarification: I mean the grpcio-tools corresponding to the oldest compatible grpcio. The package doesn't actually depend on grpcio-tools, but it includes files generated by grpcio-tools, and those require the corresponding grpcio version or newer.
There was a problem hiding this comment.
What a pain. I'd rather not be stuck using an old Python version for non-test steps. That's likely to accumulate tech debt.
Why should we go out of our way to avoid an update to protobuf and grpcio if the new versions are still compatible with the files generated by the old version of grpcio-tools?
There was a problem hiding this comment.
What a pain. I'd rather not be stuck using an old Python version for non-test steps. That's likely to accumulate tech debt.
Do the nimi-python build/codegen scripts need features or tools that are not available in Python 3.9?
It looks like you last updated the build/codegen scripts to use Python 3.8+ syntax: #2043
I would try it myself and see, if nimi-python used mainstream Python tooling such as Poetry, but I don't want to bother setting up WSL, GNU make, etc. Last time I tried that, I ran into conflicts between pyenv on Linux and pyenv-win on Windows.
Why should we go out of our way to avoid an update to protobuf and grpcio if the new versions are still compatible with the files generated by the old version of grpcio-tools?
I have been avoiding this for the packages I maintain because they don't need any new protobuf/grpcio features, so it seems like updating the version constraints doesn't provide any user value.
It seems weird to say "Python 3.9 users, you have to upgrade to grpcio 1.67 or later because that's the first version that supports Python 3.13, which you are not using, but which we use in our build pipeline."
However, there are unpatched security vulnerabilities in older versions of these packages, so maybe it's fine to start requiring newer versions. Users who pin/lock their dependencies to use older versions of protobuf/grpcio can continue to use older versions of NI packages as well.
I guess the biggest concern is that nimi-python, nidaqmx-python, and measurement-plugin-python are still compatible with each other:
- Each of these packages contains a copy of
session_pb2.py. The serialized descriptors in these files must be identical, or users will get errors when importing more than one copy. I just tried this with your branch and the nidaqmx-python master branch and I didn't get an error, so I think it's still fine. - measurement-plugin-python currently requires
protobuf = "^4.21", so existing versions will be incompatible with the updated nimi-python. But I suppose that pip, Poetry, etc. should backtrack and install the old nimi-python in this scenario.
Cc: @mshafer-NI
There was a problem hiding this comment.
Do the nimi-python build/codegen scripts need features or tools that are not available in Python 3.9?
No, we just historically run all steps on the newest Python and run a limited set of steps on the older versions.
Until this PR, we ran all of the steps on Python 3.12, so we could continue to do that and make no changes to pb2 files and no changes to gRPC deps apart from editing/removing the protobuf upper bound. It just feels like tech debt.
However, there are unpatched security vulnerabilities in older versions of these packages, so maybe it's fine to start requiring newer versions. Users who pin/lock their dependencies to use older versions of protobuf/grpcio can continue to use older versions of NI packages as well.
Sounds like a solid reason to upgrade.
Each of these packages contains a copy of session_pb2.py. The serialized descriptors in these files must be identical, or users will get errors when importing more than one copy. I just tried this with your branch and the nidaqmx-python master branch and I didn't get an error, so I think it's still fine.
You have my attention, now. This seems like a strong reason to stick with our current version of grpcio-tools, even if our tests pass and you didn't get an error.
What does this Pull Request accomplish?
Adds official Python 3.13 support to nimi-python through testing.
The version of Python used by readthedocs for documentataion generation remains unchanged at 3.11. I prefer to be intentional about changes to the generation of documentation.
Update grpcio, grpcio-tools, protobuf deps for Python 3.13 compatibility and regenerate .pb2 files.
grpcio-tools compatibility for version 1.67.0 (The earliest version witha a 3.13 wheel)
List issues fixed by this Pull Request below, if any.
What testing has been done?
PR Checks