Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Allow turning off minor ticks on Colorbar with LogNorm#13265

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

LevN0
Copy link
Contributor

@LevN0LevN0 commentedJan 22, 2019
edited
Loading

Allow turning off minor ticks on Colorbar with LogNorm

PR Summary

Closes#13257.

Allows disabling minor ticks for Colorbar when it has LogNorm.

Before

Minor ticks can be disabled for all norms except LogNorm.

The following example is intended to show that when having arbitrary major tick marks, the minor tick marks add rather than remove confusion. Example,

fig,ax=plt.subplots()data=np.clip(randn(250,250)*1000,0,1000)cax=ax.imshow(data,norm=mpl.colors.LogNorm(vmin=1,vmax=1000))cbar=fig.colorbar(cax,format='%d',ticks=[5,50,500])

Produces,

After

Once minor ticks can be disabled even for LogNorm, calling cbar.minorticks_off() will leave only major ticks in the above figure.

Allow minorticks back on for LogNorm as well

Since minor ticks can now be turned off for LogNorm, makes sense to allow turning them back on for LogNorm too.

Previously they would be enabled inColorbarBase.update_ticks for LogNorm by default, however minorticks_off and minorticks_off was inColorbar. These have been moved, in part so thatupdate_ticks can take advantage of them.

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

Allow turning off minor ticks on Colorbar with LogNorm
@jklymak
Copy link
Member

Looks good! Can you also remove the check that prevents turning them back on?

Does anyone know why we did this in the first place?

@jklymak
Copy link
Member

#11584 (comment) Looks to just have been a misunderstanding....

Also allow minorticks_on for LogNorm
@LevN0
Copy link
ContributorAuthor

Uh oh, this is probably going to be more complicated than I thought.

It makes sense to remove the check that prevents turning ticks back on with LogNorm, but then it still needs an else. I am not sure how to do it right. Do not know why minor ticks are enabled in ColorbarBase for LogNorm but for everything else the methods are in Colorbar.

@jklymak
Copy link
Member

Do not know why minor ticks are enabled in ColorbarBase for LogNorm but for everything else the methods are in Colorbar.

That might just be a mistake?

@jklymak
Copy link
Member

I actually think the minor tick methods should go onColorbarBase. The only reason forColorbar (I think) is to add a mappable to the the colorbar.

PEP8
@NelleV
Copy link
Member

This looks good! Can you add a unit test?

@LevN0
Copy link
ContributorAuthor

LevN0 commentedJan 23, 2019
edited
Loading

Made an attempt.

Edit: I really should have just setup the environment to run pytest locally, would have saved me time in seeing the miss-named import.

Add unit tests
@LevN0LevN0force-pushed theallow-disable-minorticks-lognorm-colorbar branch from8899bae toeb411d0CompareJanuary 23, 2019 05:56
Copy link
Member

@jklymakjklymak left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks@LevN0 !

Copy link
Member

@dstansbydstansby left a comment

Choose a reason for hiding this comment

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

Thanks!

@dstansbydstansby added this to thev3.1 milestoneJan 23, 2019
@dstansbydstansby merged commit397e79e intomatplotlib:masterJan 23, 2019
@LevN0
Copy link
ContributorAuthor

Thank you everyone for the guidance, especially@jklymak.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@jklymakjklymakjklymak approved these changes

@dstansbydstansbydstansby approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.1.0
Development

Successfully merging this pull request may close these issues.

4 participants
@LevN0@jklymak@NelleV@dstansby

[8]ページ先頭

©2009-2025 Movatter.jp