Skip to content

refactor code according to pandas 1.5.0 and sklearn 1.2.0 releases - #625

Merged
solegalli merged 7 commits into
feature-engine:mainfrom
luismavs:warnings
Mar 12, 2023
Merged

refactor code according to pandas 1.5.0 and sklearn 1.2.0 releases#625
solegalli merged 7 commits into
feature-engine:mainfrom
luismavs:warnings

Conversation

@luismavs

@luismavs luismavs commented Mar 5, 2023

Copy link
Copy Markdown
Contributor

Update to fix some Warnings when running code with recent Pandas (>=1.5.0) and Scikit-Learn (>=1.2.0).

Specifically, the following types of warnings were fixed:

FutureWarning: In a future version of pandas all arguments of DataFrame.any and Series.any will be keyword-only.
    assert (X.isna().any(1) == df_enc_big_na.isna().any(1)).all()`

FutureWarning: The `inplace` parameter in pandas.Categorical.add_categories is deprecated and will be removed in a future version. Removing unused categories will always return a new Categorical object.
    X[variable].cat.add_categories(

DeprecationWarning: In a future version, `df.iloc[:, i] = newvals` will attempt to set the values inplace instead of always setting a new array. To retain the old behavior, use either `df[df.columns[i]] = newvals` or, if columns are non-unique, `df.isetitem(i, newvals)`
    X.fillna(value=self.imputer_dict_, inplace=True)

SettingWithCopyWarning: 
  A value is trying to be set on a copy of a slice from a DataFrame
  
  See the caveats in the documentation: https://pandas.pydata.org/pandas-docs/stable/user_guide/indexing.html#returning-a-view-versus-a-copy
    data["var_3"].iloc[15] = np.nan

DeprecationWarning: In a future version, `df.iloc[:, i] = newvals` will attempt to set the values inplace instead of always setting a new array. To retain the old behavior, use either `df[df.columns[i]] = newvals` or, if columns are non-unique, `df.isetitem(i, newvals)`
    X.loc[:, self.variables_] = np.power(X.loc[:, self.variables_], 1 / self.exp)

FutureWarning: `sparse` was renamed to `sparse_output` in version 1.2 and will be removed in 1.4. `sparse_output` is ignored unless you leave `sparse` to its default value.
    warnings.warn(

@solegalli

Copy link
Copy Markdown
Collaborator

Hey @luismavs

Awsome job, thank you!

We need a small fix here for style tests:

tests/test_wrappers/test_sklearn_wrapper.py:55:89: E501 line too long (94 > 88 characters)

@solegalli solegalli linked an issue Mar 6, 2023 that may be closed by this pull request
@solegalli solegalli changed the title Warnings reafactor code according to pandas 1.5.0 and sklearn 1.2.0 releases Mar 6, 2023
@luismavs luismavs changed the title reafactor code according to pandas 1.5.0 and sklearn 1.2.0 releases refactor code according to pandas 1.5.0 and sklearn 1.2.0 releases Mar 7, 2023
@solegalli

Copy link
Copy Markdown
Collaborator

Is this PR finished @luismavs ?

If you rebase main, the test docs should pass. I fixed them in #627

Alternatively, I think it is safe to merge like this as well, because I know where the error comes from and it should be fixed in main.

Just waiting for your reply either way! Thank you!

Comment thread feature_engine.code-workspace Outdated
Comment thread test_run_cut.txt Outdated
@solegalli

Copy link
Copy Markdown
Collaborator

I just noticed 2 extra files that I think helped you find the warnings. If you remove those, you can probably also rebase main and then the test would pass.

Let me know whenever you think this is ready to merge.

I've seen some discussions on linkedin about this issue from pandas. Thank you so much for picking it up! :)

@codecov

codecov Bot commented Mar 10, 2023

Copy link
Copy Markdown

Codecov Report

Merging #625 (33ada3f) into main (9db7124) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 33ada3f differs from pull request most recent head 5da5ff7. Consider uploading reports for the commit 5da5ff7 to get more accurate results

@@           Coverage Diff           @@
##             main     #625   +/-   ##
=======================================
  Coverage   97.90%   97.90%           
=======================================
  Files          98       98           
  Lines        3588     3588           
  Branches      695      695           
=======================================
  Hits         3513     3513           
  Misses         28       28           
  Partials       47       47           
Impacted Files Coverage Δ
feature_engine/imputation/base_imputer.py 100.00% <100.00%> (ø)
feature_engine/imputation/categorical.py 95.31% <100.00%> (+0.07%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@luismavs

luismavs commented Mar 10, 2023

Copy link
Copy Markdown
Contributor Author

I just noticed 2 extra files that I think helped you find the warnings. If you remove those, you can probably also rebase main and then the test would pass.

Let me know whenever you think this is ready to merge.

I've seen some discussions on linkedin about this issue from pandas. Thank you so much for picking it up! :)

Hi, files removed and code rebased from main, it should be ok now.

@solegalli

Copy link
Copy Markdown
Collaborator

Hey @luismavs

Not sure what went wrong, but something broke with the last commits. Could you have a look?

I'll really appreciate it :)

@solegalli

Copy link
Copy Markdown
Collaborator

closing and reopening to see if tests pass

@solegalli solegalli closed this Mar 12, 2023
@solegalli solegalli reopened this Mar 12, 2023
@solegalli

Copy link
Copy Markdown
Collaborator

Hey @luismavs ignore my comment, it was some failure to load a dataframe from an url, for another unrelated test.

Merging as we speak. Thanks a lot for the contribution!

@solegalli
solegalli merged commit ab91403 into feature-engine:main Mar 12, 2023
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.

Avoid Pandas inplaces

2 participants