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

[MRG] Fix FutureWarning in plot_partial_dependence_visualization_api.py #16256

Conversation

ksslng
Copy link
Contributor

@ksslng ksslng commented Jan 28, 2020

Reference Issues/PRs

This PR fixes a Warning in of the examples of #14117.
Specifically examples/plot_partial_dependence_visualization_api.py (FutureWarning).

What does this implement/fix? Explain your changes.

Attribute classes_ in DecisionTreeRegressor has been moved to super class BaseDecisionTree and is deprecated. With version 0.24 it will be removed. With an isintance check a hasattr call is prevented and solves the warning, which can be removed in version 0.24.

Any other comments?

Removed also from ..tree._tree import DTYPE as it's unused and causes a linting complain.

Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM, just a small suggestion on making the inline comment more explicit:

sklearn/inspection/_partial_dependence.py Outdated Show resolved Hide resolved
Copy link
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

Thanks a lot @ksslng !

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Maybe we can just check for isclassifier() first? LGTM anyway

@ksslng
Copy link
Contributor Author

ksslng commented Jan 29, 2020

@NicolasHug thank you for the hint. I hope it's good now!

@ogrisel ogrisel merged commit 3109add into scikit-learn:master Jan 29, 2020
@ogrisel
Copy link
Member

ogrisel commented Jan 29, 2020

Thanks @ksslng!

@ksslng ksslng deleted the fix-future-warning-in-example-plot-partial-dependence-visualization-api branch January 29, 2020 20:19
thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Feb 22, 2020
panpiort8 pushed a commit to panpiort8/scikit-learn that referenced this pull request Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants