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 Python 3.13 Support #2080

Merged
merged 10 commits into from
Feb 18, 2025
Merged

Conversation

ni-jfitzger
Copy link
Collaborator

@ni-jfitzger ni-jfitzger commented Feb 5, 2025

  • This contribution adheres to CONTRIBUTING.md.
  • I've updated CHANGELOG.md if applicable.
  • I've added tests applicable for this pull request

What does this Pull Request accomplish?

  1. Adds official Python 3.13 support to nimi-python through testing.
    The version of Python used by readthedocs for documentation generation remains unchanged at 3.11. I prefer to be intentional about changes to the generation of documentation.

  2. Update grpcio, protobuf deps for Python 3.13 compatibility

Initially attempted codegen with Python 3.13 and chose grpcio, protobuf versions for compatibility with the newer grpcio-tools version. We decided to continue generating pb2 files with Python 3.12 to avoid breaking compatibility with other NI Python packages. See this discussion. Afterward, I kept the grpcio, protobuf versions in our tox .inis because I knew they were compatible with Python 3.13 and can be used if/when we begin code-generating with Python 3.13.

grpcio-tools compatibility for version 1.67.0 (The earliest version with a 3.13 wheel)

Due to generating with the same grpcio-tools version as before, I had to disable the ensure_codegen_up_to_date check on Python 3.13, though it still runs on the other versions.

List issues fixed by this Pull Request below, if any.

What testing has been done?

PR Checks

@codecov-commenter
Copy link

codecov-commenter commented Feb 5, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.32%. Comparing base (dd57eae) to head (d02180c).
Report is 3 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2080      +/-   ##
==========================================
- Coverage   91.33%   91.32%   -0.02%     
==========================================
  Files          66       66              
  Lines       16274    16274              
==========================================
- Hits        14864    14862       -2     
- Misses       1410     1412       +2     
Flag Coverage Δ
codegenunittests 84.95% <ø> (ø)
nidcpowersystemtests 94.59% <ø> (+0.04%) ⬆️
nidcpowerunittests 89.53% <ø> (ø)
nidigitalsystemtests 92.20% <ø> (ø)
nidigitalunittests 68.45% <ø> (ø)
nidmmsystemtests 92.72% <ø> (ø)
nifakeunittests 86.98% <ø> (-0.26%) ⬇️
nifgensystemtests 94.86% <ø> (ø)
nimodinstsystemtests 73.85% <ø> (ø)
nimodinstunittests 94.20% <ø> (ø)
niscopesystemtests 92.94% <ø> (ø)
niscopeunittests 43.20% <ø> (ø)
nisesystemtests 91.50% <ø> (ø)
niswitchsystemtests 82.03% <ø> (ø)
nitclksystemtests 94.87% <ø> (ø)
nitclkunittests 98.26% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd57eae...d02180c. Read the comment docs.

@ni-jfitzger
Copy link
Collaborator Author

ni-jfitzger commented Feb 16, 2025

A crash in a test?

Windows fatal exception: code 0xe0434f4e
... File "C:\ar_work\nimi-python\nimi-python\src\nidigital\system_tests\test_system_nidigital.py", line 1245 in test_specifications_levels_and_timing_multiple

https://github.com/ni/nimi-python/actions/runs/13359250170/job/37306365454

@ni-jfitzger ni-jfitzger marked this pull request as ready for review February 17, 2025 13:02
@ni-jfitzger ni-jfitzger requested a review from bkeryan February 17, 2025 21:44
Copy link
Contributor

@bkeryan bkeryan left a comment

Choose a reason for hiding this comment

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

Approved with suggestions

@ni-jfitzger
Copy link
Collaborator Author

@marcoskirsch
This is tested and has approvals from Brad and Tobias, so I'm submitting it without further review to avoid taking up your time.
There are no API changes, so it's less important to have your scrutiny.

@ni-jfitzger ni-jfitzger merged commit 075ce82 into ni:master Feb 18, 2025
34 of 35 checks passed
@ni-jfitzger ni-jfitzger deleted the add-python-3.13-support-2 branch February 18, 2025 19:57
@ni-jfitzger ni-jfitzger added this to the nimi-python 1.4.9 milestone Feb 25, 2025
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.

Add Python 3.13 support and loosen protobuf version constraint
3 participants