-
Notifications
You must be signed in to change notification settings - Fork 32
Fix tests for compatibility with Python 3.14 #3190
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3190 +/- ##
=======================================
Coverage 99.90% 99.90%
=======================================
Files 376 376
Lines 53157 53158 +1
=======================================
+ Hits 53105 53106 +1
Misses 52 52 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Impressive results, but the LFRic extraction fails while using fab? So maybe one for @hiker to look at. The log shows: (which we already had before) But also: The process still continues and finally fails with: |
|
I tried reproducing the error by executing what's in here for one of the given files and... it worked :/ In any case, the good news is it's fparser throwing the exception... Unfortunately, I noticed that running the last command on that snippet more than once crashes - I'm not sure if that's related - but I'm also inclined to say that's not intended behaviour? |
|
This is probably intended (we pass a reader stream that has already been consumed), but regardless it also fails with older python interpreters. So it is not the issue here I believe. |
Ah, I see, it makes good sense if it behaves like a stream: maybe the name should reflect that :P I'm tempted to run the tests serially, that's the only thing I can think of... |
|
So, just to leave in writing what we discussed verbally earlier. This is ready to go, it bumps the python version to 3.14 for all tests except for the LFRic extraction tests that use fab, that's now the subject of #3192. |
sergisiso
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.
@nmnobre This has passed the CI and all integration tests, I will bring it up to date and merge.
pytest seems to be twice as fast?! O.o
I've left python 3.9 and updated the docs to reflect that, though it'll be EOL in two weeks (as agreed in #3043).