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

Support Pandas 2 #131

Merged
merged 13 commits into from Jun 13, 2023
Merged

Support Pandas 2 #131

merged 13 commits into from Jun 13, 2023

Conversation

Flix6x
Copy link
Collaborator

@Flix6x Flix6x commented Apr 3, 2023

This should get us ready for upgrading to Pandas 2.

The good people of sktime are still working on compatibility with Pandas 2. Their latest release (0.17.1) already resolves any problems caught by our tests, but their pyproject.toml still suggests pandas<2.0.0.

@Flix6x Flix6x added the dependencies Pull requests that update a dependency file label Apr 3, 2023
@Flix6x Flix6x self-assigned this Apr 3, 2023
@Flix6x Flix6x marked this pull request as ready for review April 20, 2023 08:12
@Flix6x Flix6x requested a review from nhoening April 20, 2023 08:20
@Flix6x
Copy link
Collaborator Author

Flix6x commented Apr 20, 2023

Good to know: our main test suite doesn't rely on sktime and in this PR automatically tested against pandas==2.0.0. Our forecast test suite does rely on sktime and tested against pandas==1.5.3.

@nhoening
Copy link
Contributor

nhoening commented May 4, 2023

Any tips on reviewing this?
Should I consult the 2.0 release notes?

@Flix6x
Copy link
Collaborator Author

Flix6x commented May 4, 2023

No tips really.

I was happy to have our tests pass. Let me know if I need to check against breaking changes explicitly.

@nhoening
Copy link
Contributor

nhoening commented May 5, 2023

Which tests? Only the ones here, or also FlexMeasures?

@nhoening
Copy link
Contributor

nhoening commented May 5, 2023

Maybe we can use this PR to update the Python version we support, as well? This is the highest:

"Programming Language :: Python :: 3.9",

But I guess 3.10 or even 3.11 are also supported.

@Flix6x
Copy link
Collaborator Author

Flix6x commented May 5, 2023

Which tests? Only the ones here, or also FlexMeasures?

Just the ones here. FlexMeasures still needs some attention, but that's a separate issue (and different repo), right?

@Flix6x
Copy link
Collaborator Author

Flix6x commented May 5, 2023

Maybe we can use this PR to update the Python version we support, as well?

We test 3.10, but not 3.11. So I think I'd just add 3.10 for now.

@nhoening
Copy link
Contributor

nhoening commented May 5, 2023

Well it would add confidence here. But of course if it requires more work there, we need to see how we can manage.

@Flix6x
Copy link
Collaborator Author

Flix6x commented May 8, 2023

I tried it out (FM with Pandas 2), and found 1 more issue that timely-beliefs tests should have caught. I'll expand the test suite accordingly, and implement a fix (or workaround, because I believe it might be a regression in Pandas).

@nhoening
Copy link
Contributor

nhoening commented May 8, 2023

You will do that here in this PR right?

Flix6x added 5 commits May 8, 2023 11:38
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Copy link
Collaborator Author

Flix6x commented May 8, 2023

It turned out that the DataFrame constructor still needs to call finalize in order to propagate the metadata, so I reverted the version check for said constructor.

It is only the Series constructor that can no longer call finalize, otherwise I get a RecursionError: maximum recursion depth exceeded while calling a Python object.

@Flix6x Flix6x changed the title Support pandas 2.0.0 Support Pandas 2 May 8, 2023
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

Looks good, but I have one question.

timely_beliefs/tests/test_belief_utils.py Show resolved Hide resolved
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x merged commit c6c38b1 into main Jun 13, 2023
1 check passed
@Flix6x Flix6x deleted the support-pandas-2.0.0 branch June 13, 2023 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants