-
Notifications
You must be signed in to change notification settings - Fork 7
Support Python3.14 #209
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
base: main
Are you sure you want to change the base?
Support Python3.14 #209
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #209 +/- ##
=======================================
- Coverage 93.3% 93.3% -0.1%
=======================================
Files 256 256
Lines 10918 10932 +14
=======================================
+ Hits 10193 10200 +7
- Misses 725 732 +7
🚀 New features to boost your workflow:
|
ff544df to
4d18057
Compare
6d7984f to
01072e2
Compare
bb25f97 to
310e862
Compare
310e862 to
aae1542
Compare
0ed907a to
36e8ce6
Compare
36e8ce6 to
01cb886
Compare
| run: poetry add pandas@${{ matrix.pandas-version }} [email protected] --lock | ||
|
|
||
| - name: Install pandera from GitHub for Python 3.14 supported | ||
| run: poetry add git+https://github.com/unionai-oss/pandera.git |
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 how we want to handle this. It's mainly here to show that everything works now. I've asked the pandera maintainers if they have an ETA for a next release, but no reply so far. We can merge like this and keep it in mind (our dependencies already allow the next pandera release whenever it drops) for potential issues being opened, or we wait until we have the release and merge after dropping the "TEMPORARY" commit.
| # NOTE Somehow, the dtype of "step_datetime" and some other columns | ||
| # (datetime64[ns]) gets converted to "object" in pd.DataFrame(); convert back | ||
| if self.columns: | ||
| for name in {"step_datetime", "created_at", "updated_at"}: | ||
| if name in self.columns: | ||
| df[name] = pd.to_datetime(df[name]) |
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.
I'm not sure why we had this construct of setting columns to themselves as different types, and it might not matter anyway with the big rewrite, but I found this erroneous: in some iamc tests ("test_run_datapoints_raw" or so), this function would receive a step_datetime columns of datetime64[ns] type, but find a dtype of object stored for it in self.dtypes, effectively replacing the correct type with a generic one.
This solution uses hardcoded names, which is not ideal either, but keeps the dtype correct.
Though it does retain the ChainedAssignmentError warning that it was supposed to address: the CI at least still shows this warning for line 145, but even though I use the same versions of pandas and pydantic locally, I can't replicate this (I do use Python 3.12 locally, not 3.14).
Using df.loc[:, name] = pd.to_datetime(df.loc[:, name]) does not resolve this warning, unfortunately.
But maybe the rewrite doesn't rely so much on these exact forms of assignment :)
01cb886 to
b711f72
Compare
This PR aims to add support for Python 3.14. Since this already works in genno, we might also manage here already :)
In its structure, the PR is analogous to #132. Though when I ran
poetry checkto confirm the lock file was up to date withpyproject.toml, I received numerous DeprecationWarnings about poetry 2.0 having moved from thetool.poetrysection to the standardproject(see their docs for more details). Thus I took this opportunity to perform the migration, too.