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

Fixes for Pandas 2 support #742

Merged
merged 49 commits into from
Feb 4, 2025
Merged

Conversation

bartbroere
Copy link
Contributor

I have started to work on the Pandas 2 support again. Locally I'm testing with an environment that already uses Pandas 2, and these are the changes that were required to get some tests fixed.

The objective is to not break the Pandas 1.5 support, so these should be safe to merge. Where it seemed useful I added some line comments with the reasoning behind the change.

Some more PRs might follow later this week. Once all tests pass on Pandas 2, we can update the CI to include it in the test matrix.

@pquentin
Copy link
Member

Thank you for this! Will review later this week.

The objective is to not break the Pandas 1.5 support, so these should be safe to merge. Where it seemed useful I added some line comments with the reasoning behind the change.

It's OK to break Pandas 1.5 support in a new release if it brings Pandas 2.0 support.

@bartbroere bartbroere changed the title Several fixes for the tests to prepare for Pandas 2 support Fixes for Pandas 2 support Dec 19, 2024
@pquentin
Copy link
Member

pquentin commented Jan 6, 2025

buildkite test this please

Copy link
Contributor Author

@bartbroere bartbroere left a comment

Choose a reason for hiding this comment

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

I agree with all the suggestions, so I'll address them one at a time and close the corresponding threads.

eland/etl.py Outdated Show resolved Hide resolved
eland/field_mappings.py Outdated Show resolved Hide resolved
eland/operations.py Outdated Show resolved Hide resolved
tests/common.py Outdated Show resolved Hide resolved
eland/operations.py Outdated Show resolved Hide resolved
@pquentin
Copy link
Member

buildkite test this please

eland/dataframe.py Outdated Show resolved Hide resolved
@bartbroere
Copy link
Contributor Author

@pquentin I addressed all open comments, and I think the pull request is ready for re-review. Feel free to re-open comments if something needs to be changed.

@bartbroere bartbroere requested a review from pquentin February 3, 2025 10:46
@pquentin
Copy link
Member

pquentin commented Feb 3, 2025

buildkite test this please

Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! I left two nitpicks, also lint is failing.

eland/dataframe.py Show resolved Hide resolved
eland/dataframe.py Show resolved Hide resolved
eland/etl.py Outdated Show resolved Hide resolved
tests/notebook/test_demo_notebook.ipynb Show resolved Hide resolved
@pquentin
Copy link
Member

pquentin commented Feb 4, 2025

buildkite test this please

Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM. 🚀

@pquentin pquentin merged commit 75c57b0 into elastic:main Feb 4, 2025
4 checks passed
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.

2 participants