MNT Ignore pandas 2.2 warnings including Pyarrow dependency warnings #28305

lesteve · 2024-01-29T10:34:18Z

Maybe slightly simpler alternative to #28258. I think I found a way to ignore the warning in sklearn/conftest.py. This is a bit hacky (yet another place for warning handling on top of setup.cfg and CI test_script.sh), but this may be a reasonable trade-off in the short to medium term until the pandas Pyarrow dependency is a bit more clear.

github-actions · 2024-01-29T10:35:31Z

✔️ Linting Passed

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

_{Generated for commit: f5ec234. Link to the linter CI: here}

adrinjalali · 2024-01-29T10:38:05Z

This is spreading warning ignore code even more in different places. I like that in #28258 they're all moving to the same place.

lesteve · 2024-01-29T10:40:59Z

Yes completely agreed on yet another place for handling warnings, I was editing my message at the same time 😉.

IMO we spent already way too much time for something that should have been easy (ignoring a warning).

I am kind of hoping that the pandas situation improves in the short-term, either they remove the newline in the warning which makes it easy to ignore with -W or they go back on requiring Pyarrow, or something else.

adrinjalali · 2024-01-29T12:24:56Z

Yes, but I really don't want to add more tech-debt here for warnings.

adrinjalali · 2024-01-29T12:57:10Z

wanna push to my branch instead? we're doing the same thing it seems 😁 and I'm -1 on ignoring the warning this way.

lesteve · 2024-01-29T13:31:21Z

Personally I would avoid running locally with warnings turned into errors. It could lead to contributors (including us) finding some combination of packages versions that create warnings and us to chase not so important things (as we are doing right now IMO 😉) ...

I would prioritise unlocking the situation to get to up-to-date lock-files with a green CI and that issues can be solved separately and things can be cleaned-up soonish

pandas 2.2 warnings (actually this was done in this PR)
pytest 8 behaviour changes
python 3.12 datetime.datetime warnings (I think you are seeing them in your branch somehow, this has been known locally too for some time)
having cython 3.0.8 as the min version

adrinjalali · 2024-01-29T14:08:42Z

Everytime we do a "quick fix" on the CI, next time it takes longer, and the bus factor goes lower. Our CI is already quite complicated, and I'm trying to make it a bit easier for all of us to understand it and replicate it locally.

lesteve · 2024-01-29T14:34:11Z

I don't know about this, I am cleaning lock-files regularly for temporary work-arounds, maybe I am the only one that cares but I still do it 😉 ...

lesteve · 2024-01-29T15:03:03Z

I'm trying to make it a bit easier for all of us to understand it and replicate it locally.

Yeah, I agree that this is definitely something we want!

lesteve · 2024-01-29T15:52:15Z

In the meantime, CI is green 🎉. I'd try to look at #28258 to see what can be back-ported, it seems at least some tweaks in examples to avoid pandas 2.2 warnings.

adrinjalali

I'm -1 on this as a permanent solution, but happy to merge this, and try to fix #28258 tomorrow and merge that one. And I don't think the pandas issue is going away soon.

adrinjalali · 2024-01-29T18:18:49Z

build_tools/update_environments_and_lock_files.py

@@ -80,7 +80,10 @@

 docstring_test_dependencies = ["sphinx", "numpydoc"]

-default_package_constraints = {}
+default_package_constraints = {
+    # XXX: Temporary work-around for pytest


link to an issue for what's happening?

Comment added with reference to Pytest PR. IMO this change makes sense, we need to tweak slightly our tests to take it into account.

adrinjalali · 2024-01-29T18:20:22Z

examples/applications/plot_cyclical_feature_engineering.py

+X["weather"] = (
+    X["weather"]
+    .astype(object)
+    .replace(to_replace="heavy_rain", value="rain")
+    .astype("category")
+)


Or

Suggested change

X["weather"] = (

X["weather"]

.astype(object)

.replace(to_replace="heavy_rain", value="rain")

.astype("category")

)

X.loc[X["weather"] == "heavy_rain", "col"] = "rain"

X["weather"] = X["weather"].astype("category").cat.remove_unused_categories()

but don't know which one's better

I don't like mine, but I haven't found anything better after looking for 20 minutes on how to merge two categorical values ...

I find the flow is more readable somehow than your suggestion. I agree it is not super clear why you need .astype(object) and then .astype('category'). If you want to know: avoid yet another pandas warning, replace is not supposed to change categories.

Actually I found this which maybe is a bit simpler?

X["weather"] = X["weather"].cat.remove_categories("heavy_rain").fillna("rain")

if there are already missing values, this removes them though. So I don't think we'd want to encourage the pattern in our examples.

Hmmm good point actually ... I reverted this commit and went with my first proposal

FYI: pandas-dev/pandas#57104

This reverts commit 55932b5.

lesteve · 2024-01-30T10:22:05Z

FYI, I have some local fixes for Pytest 8, and I will open a PR with them and the lock-file update, when this PR gets merged.

adrinjalali

Let's merge this, and then we need to figure out why the debian run on my PR fails.

glemaitre

Let's go with this version because it is green.

glemaitre · 2024-01-31T10:23:47Z

Thanks @lesteve

…cikit-learn#28305) Co-authored-by: adrinjalali <adrin.jalali@gmail.com>

…28305) Co-authored-by: adrinjalali <adrin.jalali@gmail.com>

lesteve and others added 6 commits January 29, 2024 11:26

MNT Ignore pandas Pyarrow DeprecationWarning

e996958

relax tolerance

7bf6573

Update lock files

2bd6c6c

ignore warning for circleci

fa19370

Tweak warning

e201036

[azure parallel] [doc build]

eb39091

github-actions bot added the module:covariance label Jan 29, 2024

lesteve changed the title ~~MNT Ignore pandas Pyarrow DeprecationWarning in sklearn/conftest.py~~ MNT Yet another way to ignore pandas Pyarrow DeprecationWarning Jan 29, 2024

lesteve mentioned this pull request Jan 29, 2024

MNT Ignore pandas deprecation warning for PyArrow #28258

Closed

lesteve added the No Changelog Needed label Jan 29, 2024

Pin pytest<8

4292555

lesteve added 2 commits January 29, 2024 14:16

[doc build]

a1b4767

[azure parallel] [doc build]

2c7f18d

[doc build] more pandas 2.2 fixes

25471aa

adrinjalali reviewed Jan 29, 2024

View reviewed changes

lesteve added 2 commits January 29, 2024 22:36

Add comment

21fd668

[doc build] maybe simplification categories replacement

55932b5

lesteve mentioned this pull request Jan 30, 2024

⚠️ CI failed on Wheel builder ⚠️ #28302

Closed

Revert "[doc build] maybe simplification categories replacement"

f5ec234

This reverts commit 55932b5.

lesteve changed the title ~~MNT Yet another way to ignore pandas Pyarrow DeprecationWarning~~ MNT ignore pandas 2.2 warnings, including Pyarrow dependency warnings Jan 30, 2024

lesteve changed the title ~~MNT ignore pandas 2.2 warnings, including Pyarrow dependency warnings~~ MNT Ignore pandas 2.2 warnings, including Pyarrow dependency warnings Jan 30, 2024

adrinjalali approved these changes Jan 31, 2024

View reviewed changes

lesteve changed the title ~~MNT Ignore pandas 2.2 warnings, including Pyarrow dependency warnings~~ MNT Ignore pandas 2.2 warnings including Pyarrow dependency warnings Jan 31, 2024

glemaitre approved these changes Jan 31, 2024

View reviewed changes

glemaitre merged commit bb87768 into scikit-learn:main Jan 31, 2024

lesteve deleted the ignore-pandas-pyarrow-warning branch January 31, 2024 11:00

glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Feb 10, 2024

MNT Ignore pandas 2.2 warnings including Pyarrow dependency warnings (s…

ebd8aec

…cikit-learn#28305) Co-authored-by: adrinjalali <adrin.jalali@gmail.com>

glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Feb 13, 2024

MNT Ignore pandas 2.2 warnings including Pyarrow dependency warnings (s…

8801358

…cikit-learn#28305) Co-authored-by: adrinjalali <adrin.jalali@gmail.com>

glemaitre pushed a commit that referenced this pull request Feb 13, 2024

MNT Ignore pandas 2.2 warnings including Pyarrow dependency warnings (#…

1fe618c

…28305) Co-authored-by: adrinjalali <adrin.jalali@gmail.com>

Uh oh!

MNT Ignore pandas 2.2 warnings including Pyarrow dependency warnings #28305

MNT Ignore pandas 2.2 warnings including Pyarrow dependency warnings #28305

Uh oh!

Conversation

lesteve commented Jan 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

adrinjalali commented Jan 29, 2024

Uh oh!

lesteve commented Jan 29, 2024

Uh oh!

adrinjalali commented Jan 29, 2024

Uh oh!

adrinjalali commented Jan 29, 2024

Uh oh!

lesteve commented Jan 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adrinjalali commented Jan 29, 2024

Uh oh!

lesteve commented Jan 29, 2024

Uh oh!

lesteve commented Jan 29, 2024

Uh oh!

lesteve commented Jan 29, 2024

Uh oh!

adrinjalali left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adrinjalali Jan 29, 2024

Choose a reason for hiding this comment

Uh oh!

lesteve Jan 29, 2024

Choose a reason for hiding this comment

Uh oh!

adrinjalali Jan 29, 2024

Choose a reason for hiding this comment

Uh oh!

lesteve Jan 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lesteve Jan 29, 2024

Choose a reason for hiding this comment

Uh oh!

adrinjalali Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

lesteve Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

adrinjalali Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

lesteve commented Jan 30, 2024

Uh oh!

adrinjalali left a comment

Choose a reason for hiding this comment

Uh oh!

glemaitre left a comment

Choose a reason for hiding this comment

Uh oh!

glemaitre commented Jan 31, 2024

Uh oh!

Uh oh!

lesteve commented Jan 29, 2024 •

edited

Loading

github-actions bot commented Jan 29, 2024 •

edited

Loading

lesteve commented Jan 29, 2024 •

edited

Loading

adrinjalali left a comment •

edited

Loading

lesteve Jan 29, 2024 •

edited

Loading