Skip to content

Adding image stretch class #6016

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 1 commit into from
Closed

Conversation

fergalm
Copy link

@fergalm fergalm commented Feb 17, 2016

This is my first pull request to matplotlib, so feel free to point out what conventions I should be following but am not.

I want to solve the problem of supplying different image stretches to matplotlib's imshow command. Stretching an image maps the range of values to the range [0,1] in such a way that emphasizes certain range of input values in the color map.

Here are some examples that show different stretches that provide a different level of emphasis for the background in an astronomical photograph. The image is taken from examples/images_contours_and_fields/stretch.py in the PR.

imagestretch_example

This is a similar feature as suggested in PR #1780. That PR was rejected, as far as I can tell, because of the fear of a profusion of classes for every possible mathematical function that might be needed, and because the class needed to invert the transformation.

My proposed code solves these two problems. The base class accepts a mapping function, and is extended with convenience classes for the common cases of linear, logarithmic etc. stretches. The base class uses interpolation to perform the inverse transformation. I confess, I don't fully understand the rationale for the design of Normalize (for example, why is there an autoscale() and an autoscale_None()?), so my code may break something I didn't anticipate.

I've put the code in it's own module for convenience, but I could see it replacing the Normalize() class in matplotlib.colors if it's accepted.

@tacaswell
Copy link
Member

Can you explain how this is different than the existing log and power law Normalize classes we already have?

The histogram norm one seems very different though!

@fergalm
Copy link
Author

fergalm commented Feb 18, 2016

@tacaswell : There is now one class that gives the end user the power to define an arbitrary normalization. If you can define a function, you can easily use that function to normalize your data, instead of needing to understand the details of the base the class and writing an extension. That's a significant advantage, I think.

For example, to implement a cubic stretch (as a trivial example), a user can say

f = lambda x: np.power(x, 3)
norm = ImageStretch(f)
plt.imshow(... norm=norm)

A general power law stretch is more complicated, but still many fewer lines of code than mcolors.PowerNorm

class PowerNormStretch(ImageStretch):
    def __init__(self,  gamma, vmin, vmax):
        f = lambda x: np.power(x, 3)
        super(PowerNormStretch, self).__init__(f, vmin, vmax)


gamma = 3
norm = PowerNormStretch(gamma)

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Feb 18, 2016
@tacaswell
Copy link
Member

attn @efiring

@tacaswell
Copy link
Member

The point of autoscale vs autoscale_none is to deffer setting the limits as late as possible. Many of the functions that take a Norm can also take vmin and vmax which are passed through to the norm. Internally, autoscale_none is used to only update the limits a user has explicitly passed in. Removing that is a major regression.

In all of the existing Normalize classes clipping in optional, in this it is always done.

The brute-force inversion is not a bad idea.

It looks like this is really a FuncNorm class, once the API is brought back into alignment with the existing API.

Given the size and usage of mpl we are very bound in what we can do in terms of changing established APIs simple due to the number of people who's code we will break if we do so.

The poor interaction with the color bar needs to be investigated (it is the same issue that stalled the piece-wise norm class).

@WeatherGod
Copy link
Member

I remember there having been some discussion of making it easier for
creating specialized norm classes a long time ago. I wouldn't go for what
is proposed in this PR, but having a base Norm class that takes an
arbitrary function, and then remaking our existing Norms using that
framework is quite attractive...

We could even go so far as to treat norms like we treat colormaps, i.e.,
have a dictionary of norms, and that creating a Norm requires a name that
would be used as a key in that dictionary.

On Thu, Feb 18, 2016 at 1:55 PM, Thomas A Caswell notifications@github.com
wrote:

attn @efiring https://github.com/efiring


Reply to this email directly or view it on GitHub
#6016 (comment)
.

@fergalm
Copy link
Author

fergalm commented Feb 19, 2016

This being my first first PR, I figured the odds of getting it approved were low, but it seemed like the best way of advocating for the design. If I can win people over to the basic design, I'm happy to re-write it as people identify ways it might break users code.

Internally, autoscale_none is used to only update the limits a user has explicitly passed in. Removing that is a major regression.

I'm looking at Normalize.autoscale_None(), and I can't figure out why someone would call that over Normalize.autoscale(). I must be missing something. autoscale_None() also seems to be reimplemented in each extension class (e.g LogNorm, SymLogNorm, and PowerNorm). What's the reason for that?

@tacaswell tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Aug 29, 2017
@jklymak
Copy link
Member

jklymak commented May 17, 2018

This should definitely be handled by Norms. Whether an arbitrary norm exists to do what is requested here or not is a different question (though there are a few PRs to do exactly that). Thanks for the contribution though!

@jklymak jklymak closed this May 17, 2018
@story645 story645 removed this from the future releases milestone Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants