Skip to content

Removing the matplotlib decorated yticks in examples/inspection/plot_permutation_importance_multicollinear.py #19384

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

Closed
wants to merge 3 commits into from

Conversation

Bunkus1
Copy link
Contributor

@Bunkus1 Bunkus1 commented Feb 6, 2021

Reference Issues/PRs

References #19364

What does this implement/fix? Explain your changes.

I re-organized decorated yticks to stop the code from generating error so it can display the ylabels properly

Any other comments?

cc: (pair programming) @Rukaya-lab

@reshamas
Copy link
Member

reshamas commented Feb 6, 2021

#DataUmbrella sprint
cc: @Mariam-ke

@ogrisel
Copy link
Member

ogrisel commented Feb 6, 2021

I removed the decorated yticks to stop the code from generating error

But we want to keep the labels for the ticks on the horizontal bar plot for the feature importance.

Here is the result of the plot in your PR:

image

Here is how it used to display on the main branch (despite the warning):

image

You can look at the ci/circleci: doc artifact link the continuoous integration report to access the rendered HTML of your PR and compare it the the main branch (dev).

@ogrisel
Copy link
Member

ogrisel commented Feb 6, 2021

I wrote my comment above concurrently to the last commit, so it refers to the previous state of this PR.

@Bunkus1
Copy link
Contributor Author

Bunkus1 commented Feb 6, 2021

@ogrisel thanks, I took note of it

@Bunkus1
Copy link
Contributor Author

Bunkus1 commented Feb 7, 2021

Please @ogrisel, was the commit I made ok?

@glemaitre
Copy link
Member

Uhm now I see that you did not properly create the branch in #19385.

I would advise to close this PR and make the address the changes in #19385

For your future PR, be sure to always create a new branch from the main branch to propose a change. In your case, the changes of #19385 were added on the top of this branch (Hope that I made the explanations understandable).

@glemaitre
Copy link
Member

I am closing this PR in favour of #19385

@glemaitre glemaitre closed this Feb 7, 2021
@Bunkus1
Copy link
Contributor Author

Bunkus1 commented Feb 7, 2021

understood

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.

4 participants