Document auto-init behavior of colors.Normalize and cm.ScalarMappable. #3628
Conversation
modified: colors.py
norm = Normalize() # No parameters, the defaults.
aa = norm(a) # a is normalized according to a's min and max.
bb = norm(b) # b is normalized according to *a's min and max*.
That is, if vmin and vmax aren't set by the time the first __call__
happens, they are initialized as a side-effect, based on the data in
the first call.
This may well be the intended behavior, but the existing docstring
suggested autoscaling to whatever data comes through. Side-effects
should be documented so they sound like side-effects.
Documented this in the __init__ docstring (which is where the __call__
behavior was documented), and added a docstring to the __call__
method and also mentioned this side-effect behavior there.
modified: cm.py in class ScalarMappable
def __init__(norm=None, ...)
Added to the docstring that the default is really a new Normalize
initialized with no parameters, which auto-initializes scaling based
on the first input data.
|
Btw, is there a reason why ScalarMappable is still an old-style Python class? It makes it harder to find its subclasses, for one thing. Maybe I should ask, please link to the old conversation thread that discusses this to death. |
| """ | ||
| ``norm.__call__(value) <==> norm(value)`` | ||
|
|
||
| Normalize data in the ``[vmin, vmax]`` interval into the |
There was a problem hiding this comment.
Just a note: For some Norms, I believe the return type is an integer not a float. At which point, the cmap allows you to index colours.
There was a problem hiding this comment.
Grr, yes. I don't want to give wrong info. I'll look how the other docs talk around this.
Edit later: For Normalize per se say the output range is [0.0 .. 1.0], so I'm leaving the comments on Normalize just saying that.
I'd suggest doing it in a new PR and seeing if the discussion gets out of hand 😉 - I imagine you will either get a very good reason why we can't do it, or it will just be merged... |
|
The error is not related. That is due to #3598 I should really try to fix that soon. |
|
It is going to be a new style class in any case in Python 3 so hopefully it does work as a new style class |
| @@ -902,6 +904,15 @@ def process_value(value): | |||
| return result, is_scalar | |||
|
|
|||
| def __call__(self, value, clip=None): | |||
| """ | |||
| ``norm.__call__(value) <==> norm(value)`` | |||
There was a problem hiding this comment.
I don't think we need to document the python data-model.
There was a problem hiding this comment.
I was copying how Python's own classes document their "dunder" methods. The few examples I see in Numpy and Matplotlib don't do it, so I'll take that bit out.
modified: cm.py in ScalarMappable.__init__
"If not given or *None*," ==> "If *None*,"
modified: colors.py in Normalize.__call__
Removed this: "``norm.__call__(value) <==> norm(value)``"
Mentioned that __call__ takes *value* and returns
the normalized data.
|
I tested that the cm and colors modules can be imported with my changed/added docstrings. I don't believe the Travis build errors are related to my docstrings. I didn't trust myself to merge the two commits (and a merge) into one. I was surprised how many additional commits were in the merge since I forked last week. |
|
The current error at 0385071 is a pep8 style violation. Looks like you have added some trailing whitespaces which should be removed. It is normally better to rebase onto current master and force push rather than merge master into your branch. That avoids an additional empty merge commit when merged back into master. |
|
late comment re new-style class, please make a new PR to make that change. I suspect it uses the old-style classes because it was written before new-style classes existed and never got updated. |
|
|
Sorry about the jargon |
Ditto. If this is your first PR (pull request 😉) welcome aboard - its great to have your contribution! |
|
Hey folks (@tacaswell), because I did this change on my master branch, and it's not merged, I'm a little stuck as far as creating another PR for that old-style class (or anything else on matplotlib). So I have a choice:
My preference is 1). Does anyone else have a preference? |
DOC : Document auto-init behavior of colors.Normalize and cm.ScalarMappable.
|
I just merged this PR which solves the problem. You will might have to do a |
|
#3662 is "Make all classes new-style." |
modified: colors.py in class Normalize
modified: cm.py in class ScalarMappable