Skip to content

MNT bump min pandas version to 1.2.0 #30613

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

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

StefanieSenger
Copy link
Contributor

@StefanieSenger StefanieSenger commented Jan 8, 2025

This PR sets the minimum pandas version to "1.2.0", since the previous minimal version ("1.1.5") was released over four years ago and was causing some issues for _array_api._convert_to_numpy().

See these problems roughly fixed in a commit and this comment..

(Note that these issues might be more complicated once we would test better for it, every time _convert_to_numpy() would be called on an input before check_array() comes into play.

@adrinjalali, would you like to have a look?

else:
cat_column = cat_column.ravel()
# Insert extra column as a non-numpy-native dtype:
cat_column = pd.Categorical(cat_column.ravel())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pd.Categorical was introduced in pandas version 0.15.0. :)

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

Yeah that happens that we don't clean-up these ... these days I guess we would leave a TODO note with an explanation when to simplify the code again. Even then, I am pretty sure (especially in tests) there are a lot of stuff that we haven't cleaned up.

Part of me is like 😱, part of me is like 🤷. Generally 🤷 wins because it's less effort.

You could imagine that if it's easy to find these non-cleaned part of code, it could be a source for "good first issue" PRs maybe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did enjoy that cleanup. It's very satisfying. ☺️

You could imagine that if it's easy to find these non-cleaned part of code, it could be a source for "good first issue" PRs maybe.

That's a good idea!
These parts might not be easy to identify with regex (without a TODO), but a few people might find something by looking by hand (eyes) in our next sprint in February. I will point them to it.

Copy link

github-actions bot commented Jan 8, 2025

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: e490c40. Link to the linter CI: here

@adrinjalali adrinjalali changed the title MNT bump min pandas version 1.1.5 MNT bump min pandas version to 1.2.0 Jan 8, 2025
Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

LGTM if CI is green.

Copy link
Member

@lesteve lesteve left a comment

Choose a reason for hiding this comment

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

I am fine with this, we'll bump up the dependencies for scikit-learn 1.7 and 1.2.0 happened ~20 days after 1.1.5, so it should not have too much impact.

In a case like this I would typically update only the lock-files I care about i.e. the one with min min_dependencies in their name.

You can do something like this:

python build_tools/update_environments_and_lock_files.py --select-build 'min_dependencies'`

@lesteve
Copy link
Member

lesteve commented Jan 9, 2025

@StefanieSenger I wasn't sure whether I would ask you to revert the changes to the lock-files that are not related to min-dependencies. I ended up doing it because it makes it easier to review and is a bit safer.

I also used a [doc build] to make sure the doc-min-dependencies full build happens.

Basically in general only update the lock-files that you need for your PR and leave the rest to the weekly Monday automatic lock-files PRs.

More details about the "a bit safer" part: if you update all of the lock-files you need to remember to use the "exotic builds" commit markers like [arm], [scipy-dev], etc ... The CUDA build you need to remember to set the label to test it.

@lesteve lesteve enabled auto-merge (squash) January 9, 2025 13:48
@StefanieSenger
Copy link
Contributor Author

Yes, thanks @lesteve. Next time I will remember that.

@lesteve lesteve merged commit aa5c79e into scikit-learn:main Jan 9, 2025
29 checks passed
@StefanieSenger StefanieSenger deleted the pandas_min_version branch January 9, 2025 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants