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
Appearance settings

FIX: let colorbar extends work for PowerNorm #11610

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
Merged
merged 2 commits into from
Jul 10, 2018
Merged

FIX: let colorbar extends work for PowerNorm #11610

merged 2 commits into from
Jul 10, 2018

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Jul 9, 2018 •
edited
Loading

PR Summary

Closes #11486

PowerNorm needed similar special casing to LogNorm for extend='min' for colorbar...

import numpy as np

import matplotlib
import matplotlib.pyplot as plt
import matplotlib.colors as colors

fig, ax = plt.subplots(2, 1)

test_data1 = np.zeros((100,100), ) # data at the bottom bound of the range
disp_obj1 = ax[0].imshow (test_data1, norm=colors.PowerNorm(gamma=0.5, vmin=0.0, vmax=1.0,),)
# ************ to reproduce the bug, use the line with 'both' or 'min' extends *************
#fig.colorbar(disp_obj1, ax=ax[0], )
fig.colorbar(disp_obj1, ax=ax[0], extend='max')
#fig.colorbar(disp_obj1, ax=ax[0], extend='min')
#fig.colorbar(disp_obj1, ax=ax[0], extend='both')

test_data2 = np.ones((100,100), ) * 0.25 # all positive data that's inside the range
disp_obj2 = ax[1].imshow (test_data2, norm=colors.PowerNorm(gamma=0.5, vmin=0.0, vmax=1.0,),)
# ************ to reproduce the bug, use the line with 'both' or 'min' extends *************
#fig.colorbar(disp_obj2, ax=ax[1], )
#fig.colorbar(disp_obj2, ax=ax[1], extend='max')
fig.colorbar(disp_obj2, ax=ax[1], extend='min')
#fig.colorbar(disp_obj2, ax=ax[1], extend='both')

plt.show()

Before:


/Users/jklymak/matplotlib/lib/matplotlib/colors.py:1201: RuntimeWarning: invalid value encountered in power
  np.power(resdat, gamma, resdat)
/Users/jklymak/matplotlib/lib/matplotlib/colorbar.py:999: RuntimeWarning: invalid value encountered in true_divide
  z = np.take(y, i0) + (xn - np.take(b, i0)) * dy / db

figure_1

After:

figure_1

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant

@tacaswell tacaswell added this to the v3.0 milestone Jul 9, 2018
timhoffm
timhoffm approved these changes Jul 9, 2018
View reviewed changes
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Minor style issue.

lib/matplotlib/colorbar.py Outdated
if isinstance(self.norm, colors.LogNorm):
# If using a lognorm, ensure extensions don't go negative
if (isinstance(self.norm, colors.LogNorm) or
isinstance(self.norm, colors.PowerNorm)):
Copy link
Member

@timhoffm timhoffm Jul 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

if isinstance(self.norm, (colors.lOGNorm, colors.PowerNorm)):

@jklymak jklymak force-pushed the fix-extend-low-powernorm branch 3 times, most recently from 15209b6 to e43763a Compare July 9, 2018 20:58
dstansby
dstansby approved these changes Jul 9, 2018
View reviewed changes
lib/matplotlib/colorbar.py
@@ -802,8 +802,9 @@ def _process_values(self, b=None):

b = self.norm.inverse(self._uniform_y(self.cmap.N + 1))

if isinstance(self.norm, colors.LogNorm):
# If using a lognorm, ensure extensions don't go negative
if isinstance(self.norm, (colors.PowerNorm, colors.LogNorm)):
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this kind of stuff should be handled by the specific Norm class, instead of being "special cased" here in this code, but since this is how LogNorm is dealt with at the moment I guess this is fine as a fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm, interesting idea. This "b" is just the points that are used to make the pcolor mesh that makes the colorbar. So its a bit of a toss up where making that list should get created and how.

@dstansby
Copy link
Member

dstansby commented Jul 9, 2018

Looks like this is failing on the pytz tests, @pganssle any ideas what's going on with the travis build?

@jklymak
Copy link
Member Author

jklymak commented Jul 10, 2018

I think its coming from flake-8. I guess we should rerun flake8 on any new merges.

./lib/mpl_toolkits/mplot3d/axes3d.py:0:1: X100 Superfluous per-file-ignores for E303
1     X100 Superfluous per-file-ignores for E303

@jklymak jklymak force-pushed the fix-extend-low-powernorm branch from e43763a to 092e4da Compare July 10, 2018 00:29
@pganssle
Copy link
Member

pganssle commented Jul 10, 2018 •
edited
Loading

Maybe flake8 should be run on a whitelist instead of a blacklist? I didn't do the flake8 stuff, though, so I may not be much help.

@QuLogic
Copy link
Member

QuLogic commented Jul 10, 2018

pytz tests started failing on master before the flake8 issue.

@pganssle
Copy link
Member

I dunno if this is the issue to talk about it on, but it occurs to me that the tox -e pytz command can probably be folded into the tests with pandas, since pytz is a dependency of pandas anyway (... for now). If it's causing problems that's probably the most expedient way to fix it.

@tacaswell
Copy link
Member

Looks like it is a bug in an old pytest / xdist....

@jklymak jklymak closed this Jul 10, 2018
@jklymak jklymak reopened this Jul 10, 2018
@jklymak
Copy link
Member Author

jklymak commented Jul 10, 2018

The new merges let Travis pass, at least for this PR....

@dstansby dstansby merged commit efc504b into matplotlib:master Jul 10, 2018
@jklymak jklymak deleted the fix-extend-low-powernorm branch July 10, 2018 16:31
@dstansby dstansby mentioned this pull request Jan 6, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tacaswell tacaswell tacaswell approved these changes

@timhoffm timhoffm timhoffm approved these changes

@dstansby dstansby dstansby approved these changes

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

Successfully merging this pull request may close these issues.

Colorbar does not render with PowerNorm and min extend when using imshow
6 participants
@jklymak @dstansby @pganssle @QuLogic @tacaswell @timhoffm
Morty Proxy This is a proxified and sanitized view of the page, visit original site.