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.

Sign up for GitHub

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

Jump to bottom

Deperecated check_is_fitted call #570

Closed
fradav opened this issue Dec 4, 2019 · 8 comments
Closed

Deperecated check_is_fitted call #570

fradav opened this issue Dec 4, 2019 · 8 comments

Comments

@fradav
Copy link

fradav commented Dec 4, 2019

When calling GridsearchCV I got :

/home/fradav/.conda/envs/pytorch/lib/python3.7/site-packages/scikit_learn-0.22rc3-py3.7-linux-x86_64.egg/sklearn/utils/validation.py:937: FutureWarning: Passing all_or_any to check_is_fitted is deprecated and will be removed in 0.23. The any_or_all argument is ignored.

This is with scikit-learn 0.22rc3 version.

Regards,
@thomasjpfan
Copy link
Member

Can you provide a reproducible code example where the warning appears?

@BenjaminBossan
Copy link
Collaborator

We use sklearn's check_is_fitted with that parameter here:

skorch/skorch/utils.py

Line 502 in 7d275df

all_or_any=all_or_any,

Since the parameter was deprecated here, we should also remove it.

The question now is if we want to deprecate it as well or just flat out remove it. My guess is that nobody uses skorch.utils.check_is_fitted directly, which would make it safe for immediate removal. If we deprecate, people will see two deprecation warnings while just using normal skorch code, and they cannot even easily fix it.

@thomasjpfan
Copy link
Member

It still has a use case in skorch by raising skorch specific exceptions. Since it is not being used, we can change its behavior without much consequences.

A quick fix would be to check the sklearn version and make sure to call check_is_fitted with the correct arguments (depending on version).

@BenjaminBossan
Copy link
Collaborator

I just saw that the changes to check_is_fitted make it unusable for use. E.g.:

    attributes : deprecated, ignored
        .. deprecated:: 0.22
           `attributes` is deprecated, is currently ignored and will be removed
           in 0.23.

Instead, the function now only checks if there are any attributes that end on _. This is the case for skorch even before fitting.

BenjaminBossan pushed a commit that referenced this issue Dec 5, 2019
Make check_is_fitted work again
e9f35a9 
`check_is_fitted` was changed (see #570) in a way that we won't raise
a `NotInitializedError` anymore despite the net not being
initialized. This is now solved by basically porting the old
`check_is_fitted` behavior to skorch.
@BenjaminBossan BenjaminBossan mentioned this issue Dec 5, 2019
Merged
@fradav
Copy link
Author

fradav commented Dec 12, 2019

There is also :

/data/miniconda3/envs/pytorch/lib/python3.6/site-packages/sklearn/utils/deprecation.py:144: FutureWarning: The sklearn.metrics.scorer module is  deprecated in version 0.22 and will be removed in version 0.24. The corresponding classes / functions should instead be imported from sklearn.metrics. Anything that cannot be imported from sklearn.metrics is now part of the private API.
  warnings.warn(message, FutureWarning)

Should I make a separate issue for this?

@BenjaminBossan
Copy link
Collaborator

Thanks for that. Yes, please open a new issue for that.

ottonemo pushed a commit that referenced this issue Dec 16, 2019
@BenjaminBossan @ottonemo
Bugfix: sklearn 0.22 (#571)
ab8e6ed 
* Remove import-outside-toplevel from pylint

Very unnecessary :/

* Make check_is_fitted work again

`check_is_fitted` was changed (see #570) in a way that we won't raise
a `NotInitializedError` anymore despite the net not being
initialized. This is now solved by basically porting the old
`check_is_fitted` behavior to skorch.

* Fix multi_indexing with sklearn 0.22

sklearn's (_)safe_indexing used to work with tuples as indices, now
doesn't.

* Remove unnecessary import

* Make convert_sklearn_metric_function work again

In sklearn 0.22, make_scorer returns a special object that must be
treated differently in skorch.

* Update CHANGELOG
@BenjaminBossan
Copy link
Collaborator

@fradav This issue can be closed, right?

@ottonemo
Copy link
Member

I think it can be closed. @fradav please re-open the issue if you object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@fradav @thomasjpfan @BenjaminBossan @ottonemo