Skip to content

Correctly extend a lognormed colorbar #7552

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

Merged
merged 3 commits into from
Dec 3, 2016

Conversation

dstansby
Copy link
Member

@dstansby dstansby commented Dec 2, 2016

Fixes #5201.

@tacaswell
Copy link
Member

Those failures look real?

Is there a better way to do this that does not require isinstance?

@tacaswell tacaswell added this to the 2.0.1 (next bug fix release) milestone Dec 2, 2016
@dstansby
Copy link
Member Author

dstansby commented Dec 2, 2016

isinstace is used quite a bit nearby so I assumed it would be fine.

Turns out the issue is a typo (+ should be a -), in the non-lognorm code I copied into the else statement. Should be fixed now.

@efiring
Copy link
Member

efiring commented Dec 2, 2016

isinstance is fine here. This looks good to me. I think it should be merged and backported. The appveyor tests pass and Travis is simply stalled.

@efiring efiring changed the title Correctly extend a lognormed colorbar [MRG+1] Correctly extend a lognormed colorbar Dec 2, 2016
cb = ColorbarBase(ax, norm=LogNorm(vmin=0.1, vmax=1000.0),
orientation='vertical', extend='both')
assert cb._values[0] >= 0.0

Copy link
Member

Choose a reason for hiding this comment

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

One more blank line.

@cleanup
def test_colorbar_lognorm_extension():
# Test that colorbar with lognorm is extended correctly
ax = plt.gca()
Copy link
Member

Choose a reason for hiding this comment

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

Better to use plt.subplots.

@codecov-io
Copy link

Current coverage is 66.45% (diff: 100%)

Merging #7552 into master will increase coverage by 0.03%

@@             master      #7552   diff @@
==========================================
  Files           109        109          
  Lines         46780      46785     +5   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          31075      31093    +18   
+ Misses        15705      15692    -13   
  Partials          0          0          

Powered by Codecov. Last update dfd38f7...a311b96

@tacaswell
Copy link
Member

restarted travis just to be sure.

@tacaswell tacaswell changed the title [MRG+1] Correctly extend a lognormed colorbar [MRG+2] Correctly extend a lognormed colorbar Dec 3, 2016
@efiring
Copy link
Member

efiring commented Dec 3, 2016

Appveyor failures look unrelated.

@efiring efiring changed the title [MRG+2] Correctly extend a lognormed colorbar Correctly extend a lognormed colorbar Dec 3, 2016
@efiring efiring merged commit e66d2f1 into matplotlib:master Dec 3, 2016
efiring added a commit that referenced this pull request Dec 3, 2016
Correctly extend a lognormed colorbar
@efiring
Copy link
Member

efiring commented Dec 3, 2016

Backported to v2.x as 58b92b2.

@QuLogic QuLogic modified the milestones: 2.0.1 (next bug fix release), 2.0 (style change major release) Dec 7, 2016
@dstansby dstansby deleted the lognorm-colorbar-fix branch December 27, 2016 16:42
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.

5 participants