-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
WIP: Add offset normalizer #3858
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
Conversation
@tacaswell I based this off of a master that I pulled down last night. Is that ok? |
Yes On Thu, Nov 27, 2014, 11:35 Paul Hobson notifications@github.com wrote:
|
60aaec1
to
37ff5a5
Compare
You are right about my last comment, but i have still three points: os = OffsetNorm(-5, -2, -1)
x = np.linspace(-6, 6, 100)
abs(x-os.inverse(os(x))).max() #should be zero, but is two This is why my stackoverflow answer (which you probably read before :)) divides by
instead of vmax-vcenter. Second point the class has no docstring. |
Yeah. Good points all around. What you you think about |
raise ValueError("Not invertible until scaled") | ||
|
||
vmin = float(self.vmin) | ||
vcenter = float(self.vcenter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is a code path that will end up with self.vcenter
to be None
? I think you need to also override autoscale_None
to make sure it gets set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tacaswell yup. Got that written, Will default to half-way in the middle if not specified.
Thought about looking at the signs of vmax and vmin to see if 0 would be a good choice, but I think edge cases of -0.001 and 1000 make that more complicated than it means to be,
Needs documentation and tests that stress the code a bit more. |
Hey @Tillsten I really feel like your class you posted on SO is more robust. I don't feel right using it here without you on the commit log. Do you want to pull this branch down and overwrite what I have with your class? |
About the tests, happy to beef things about a bit. What do you think about the pattern I'm using with subclassing a base test case? I feel like the whole file could benefit from similar treatment (separate PR,of course). |
Here's that SO solution from @Tillsten |
@phobsen Just use it, i will hopefully get other commits anyway. If i ever find the free time ... :) |
2d41664
to
933626c
Compare
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858`
933626c
to
0b17fcf
Compare
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858`
0b17fcf
to
205e8b9
Compare
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858`
self.vmax = vmax | ||
self.clip = clip | ||
|
||
def __call__(self, value, clip=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the default value be None
?
This looks pretty solid. Left a few picky comments. I am a bit confused by the decorators working on 2.6 and a bit concerned about the test classes as I don't recall seeing them anyplace else in the test suite. |
I did. It didn't change anything, though I was working on a janky dev installation of MPL, so I should probably give it another shot. Looking back a existing source code, |
Just had an idea in terms of a name for the general case, |
It sounds like all this is actually about using a continuous piece-wise linear curve (first-degree spline) as the mapping function. Is that correct? |
|
I like that name. On Tue, Jun 30, 2015 at 11:17 PM, Till Stensitzki notifications@github.com
|
👍 |
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858`
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858`
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858`
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858`
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858`
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858`
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858`
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858`
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858`
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858` TST: add tests for DivergingNorm DOC: add to colors_api.rst
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858` TST: add tests for DivergingNorm DOC: add to colors_api.rst
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858` TST: add tests for DivergingNorm DOC: add to colors_api.rst
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858` TST: add tests for DivergingNorm DOC: add to colors_api.rst DOC: add tutorial FIX: fix extend=both
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858` TST: add tests for DivergingNorm DOC: add to colors_api.rst DOC: add tutorial FIX: fix extend=both
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858` TST: add tests for DivergingNorm DOC: add to colors_api.rst DOC: add tutorial FIX: fix extend=both DOC: add new example
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858` TST: add tests for DivergingNorm DOC: add to colors_api.rst DOC: add tutorial FIX: fix extend=both DOC: add new example
Borrows heavily from @Tillsen's solution found on StackOverflow here: http://goo.gl/RPXMYB Used with his permission dicussesd on Github here: https://github.com/matplotlib/matplotlib/pull/3858` TST: add tests for DivergingNorm DOC: add to colors_api.rst DOC: add tutorial FIX: fix extend=both DOC: add new example
Trying to close #1806
Not currently on a good machine for development, but I think this pretty straight-forward, so hopefully Travis a test runner will suffice...
OffsetNorm
: This let's you set vmin, vcenter, and vmax. The use case for this is when you have data ranging from e.g., -1, to +2, and you want the the white/gray part at 0, but would also like to use the full range of the red scale. So:Next I'll add
SymNorm
, that will look at the absolute values of vmin and vmax to produce: