Logit scale #3753
Logit scale #3753
Conversation
|
|
||
| def __init__(self, p_min, p_max): | ||
| Transform.__init__(self) | ||
| self.p_min = p_min |
There was a problem hiding this comment.
Are these intentionally part of the public api?
There was a problem hiding this comment.
I thought they are as useful as e.g. symmetric log transformations, which are already public API.
It's possible to define LogitTransform and LogisticTransform within LogitScale, but then LogitScale looks different from any other scale in that file...
There was a problem hiding this comment.
Thinking about this a bit more, they should probably be hidden behind properties so that the values can not be set to out-of-range values.
There was a problem hiding this comment.
You mean p_min and p_max? I can make a @Property around it and another _p_min/_p_max layer to hide from the user, with a setter that checks for ranges. Is that the idea?
There was a problem hiding this comment.
Yes
On Mon Nov 03 2014 at 11:49:49 AM Fabio Zanini notifications@github.com
wrote:
In lib/matplotlib/scale.py:
@@ -478,10 +478,107 @@ def get_transform(self):
return self._transform+class LogitTransform(Transform):
- input_dims = 1
- output_dims = 1
- is_separable = True
- has_inverse = True
- def init(self, p_min, p_max):
Transform.**init**(self)self.p_min = p_minYou mean p_min and p_max? I can make a @Property
https://github.com/property around it and another _p_min/_p_max layer
to hide from the user, with a setter that checks for ranges. Is that the
idea?—
Reply to this email directly or view it on GitHub
https://github.com/matplotlib/matplotlib/pull/3753/files#r19745773.
There was a problem hiding this comment.
Ok I coded this via a sister class that implements the properties. This actually cleans up constructors quite a bit.
|
I added a FIXME about having side-effects in transform_non_affine. That might be not fine, depending on how transform_non_affine is supposed to work. |
|
|
||
| @p_min.setter | ||
| def p_min(self, value): | ||
| try: |
There was a problem hiding this comment.
I lean towards letting the user handle exceptions, but this is fine.
|
A tad annoying thing about the auto-hiding line comments is that sometimes it hides active discussion. I would drop the |
|
I filed a separare PR for the **kwargs issue: PR #3756. |
|
I think now only the side effects FIXME is to be solved, then I should probably add some documentation and it's done, right? |
|
plus tests and cleaning up the pep8 violation that is making the current tests fail (https://travis-ci.org/matplotlib/matplotlib/builds/40041025) |
|
Testing is helping, I fixed a few bugs and the PEP8 offenders. I still have to:
|
|
After many pains, I decided to refactor the whole logit to make it follow closely LogScale. Now not only it does not throw any warnings, but it does not contains any arbitrary parameters like p_min. Instead, I coded a Locator class similar to LogLocator that finds out the plot range from the data. I am sorry if this forces you to recheck things, but as it happens I was finding so many little bugs here and there that the code was becoming very dirty. I did a few tests and everything seems to work over here. I am wondering where I should document the logit scale addition... could you please enlighten me on that? |
|
See https://github.com/matplotlib/matplotlib/tree/master/doc/users/whats_new for documentation on how to document the new feature. |
| @@ -14,6 +14,14 @@ def test_log_scales(): | |||
| ax.axhline(24.1) | |||
|
|
|||
|
|
|||
| @image_comparison(baseline_images=['logit_scales'], remove_text=True) | |||
| def test_logit_scales(): | |||
| ax = plt.subplot(122, yscale='logit', xscale='logit') | |||
There was a problem hiding this comment.
By that, I mean a single axes the fills up the whole figure.
There was a problem hiding this comment.
ah sry, ok let me do this. I just copied from above in the same file.
|
It looks like there are more decades on the x-axis than the y-axis, is that correct? |
|
I set nothing in particular about that, I just make sure all the data are visible. It is up to the user to specify axis limits if he wishes to. Maybe I should clarify: I follow the same design like |
|
Ok, the problem that get_minpos is kind of specially crafted for log axes stays, but I fixed the logit tick locator with a hack for now. In case limits for the axis are outside ]0, 1[, I assume 1 - get_minpos() as axis limit. That defaults to 1 - 1e-7 and should be good for most cases. I'll work on the documentation a bit now and then I hope the PR should get into a final state for acceptance. |
|
Ok I added 2 lines of documentation, is that it or should I put examples and other user friendly explanations somewhere else? |
|
Putting an example in the examples directory/gallery would be great. Finding someplace in the prose docs to write a paragraph explaining why this is useful and how to use it would be better. |
|
Also, can you sort out why all of the tests are failing? |
|
Sry ta, I made a mess, do you want me to rebase against master to make clean diffs of the whole PR changes? I don't want you to go through the 50 commits I made in the last few hours! Docs are still a bit of question marks, because I am not sure whether my text/formatting is ok. |
|
I would add a plot of some data with a known shape the test, the ticks look Over all the docs look pretty good to me, but I just skimmed them. On Fri Nov 14 2014 at 4:14:52 AM Fabio Zanini notifications@github.com
|
|
I changed the test figure to one which is by design more symmetric, so that it's evident that x/y axis follow the exact same rules. |
|
I was thinking more like one of the extinction curves we talked about when When you change the test data can you do a rebase to remove the old tests On Fri Nov 14 2014 at 1:29:47 PM Fabio Zanini notifications@github.com
|
|
I don't think this matters much, because it's a test, not an example. If anything, expecting a circle and seeing a circle is a strong indication that the feature is working, which is what tests are good for. But anyway, what curve would you like to see, I am not sure I get the meaning of "extinction curve". Something like the example plot, but without randomly generated numbers? I.e. a monotonically increasing curve or so, is that the idea? |
|
Both suggestions followed. Now I'll rebase. |
071c0b0 to
c48bacf
Compare
| @@ -478,10 +478,111 @@ def get_transform(self): | |||
| return self._transform | |||
|
|
|||
|
|
|||
| def _mask_non_logit(a): | |||
| """ | |||
| Return a Numpy masked array where all values outside ]0, 1[ are | |||
There was a problem hiding this comment.
This docstring does not match what the function is doing. It is always returning a copy of the input array in which any values outside ]0, 1[ are replaced by nan.
|
He Eric, thanks for all the comments, which I will address ASAP. |
|
The ticker module needs more work, such as #4172, so any remaining fine-tuning of the logit functionality can be done later. |
|
Cool, thanks! |
|
Hi @iosonofabio, great to this that this PR went in! I just have a question, is the logit scale also usable for the Last week I was plotting the transition matrix of a Markov chain, with many values close to 0 and many other values close to 1. Plotting the logit transform of that matrix with a diverging colormap really gave a nice result. |
|
Currently, I don't think that is possible, but there have been discussion On Mon, Jul 13, 2015 at 8:46 AM, Pierre Haessig notifications@github.com
|
|
To illustrate this notion of logit norm, I've created a Notebook example http://nbviewer.ipython.org/gist/pierre-haessig/2d4b46078d46e6fc3842 Thanks Ben for your feedback. I guess that the easiest path would be to create a new Normalize subclass and leave for fancy refactoring for another day. And it's not that clear that this would lead to that much code duplication. |
|
+1 on doing the simplest thing that will solve this problem. On Mon, Jul 13, 2015, 10:48 AM Pierre Haessig notifications@github.com
|
|
I am unfortunately terribly busy these days but I would be interested in On 13/07/2015 17:53, Thomas A Caswell wrote:
|
As discussed earlier today on the mailing list, here the proposal of PR for a new scale, the LogitScale. This is essentially a nonlinear scale that is log both towards 0+ and log towards 1-. The exact transformation is:
x -> log10 (x / 1 - x)
(the 10 base of the log is just the most common case for plotting, but I could extend to natural or base 2 logs if there is interest). The inverse transform is the logistic function:
x -> 1 / (1 + 10^-x)
(again, 10 is just one possible base).
The LogitScale is useful when one has frequencies in a population (i.e. floats between 0 and 1) and both rare events and very common ones are interesting. For instance, say you ask about the fraction of people with blue eyes in various world populations, you want to spot even tiny deviations from zero or one.
Pierre Haessig had one implementation with example plots here:
http://nbviewer.ipython.org/gist/pierre-haessig/7e3e6a818edeb6819708
I had developed another, virtually identically one in the last few days, but this PR takes essentially his code and slightly modifies it for style and a few design points.
Note: there are a few FIXME in the code in case of doubts, please enlighten me on the best practice in those cases. Tests and docs also still missing.