Document auto-init behavior of colors.Normalize and cm.ScalarMappable. #3628

switham · 2014-10-10T03:39:30Z

modified: colors.py in class Normalize

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.

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.

switham · 2014-10-10T03:46:23Z

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.

pelson · 2014-10-10T12:46:26Z

lib/matplotlib/colors.py

+        """
+        ``norm.__call__(value) <==> norm(value)``
+
+        Normalize data in the ``[vmin, vmax]`` interval into the 


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.

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.

pelson · 2014-10-10T12:47:14Z

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.

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...

jenshnielsen · 2014-10-10T12:48:06Z

The error is not related. That is due to #3598 I should really try to fix that soon.

jenshnielsen · 2014-10-10T12:55:13Z

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

tacaswell · 2014-10-10T15:04:18Z

lib/matplotlib/colors.py

@@ -902,6 +904,15 @@ def process_value(value):
        return result, is_scalar

    def __call__(self, value, clip=None):
+        """
+        ``norm.__call__(value) <==> norm(value)``


I don't think we need to document the python data-model.

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.

switham · 2014-10-15T04:58:43Z

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.

jenshnielsen · 2014-10-15T08:32:48Z

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.

tacaswell · 2014-10-15T13:32:28Z

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.

switham · 2014-10-15T17:38:33Z

I will probably not overcome my fear and ignorance of git rebase in the course of this pull request but see 4).
I think I can avoid additional merges.
I'll fix the pep 8 problems.
I see there's a piece on matplotlib workflow; I'll follow that after this pull request.
Oh! "PR" == pull request not problem report; I'll make one to fix that old-style class.

tacaswell · 2014-10-15T17:58:09Z

Sorry about the jargon

pelson · 2014-10-15T18:06:11Z

Sorry about the jargon

Ditto. If this is your first PR (pull request 😉) welcome aboard - its great to have your contribution!

switham · 2014-10-16T22:42:24Z

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:

Close this PR, reset my master, redo this PR as one commit on a branch, or
Wait for this one to be merged (hopefully) as-is before doing other stuff.

My preference is 1). Does anyone else have a preference?

DOC : Document auto-init behavior of colors.Normalize and cm.ScalarMappable.

tacaswell · 2014-10-16T23:29:20Z

I just merged this PR which solves the problem.

You will might have to do a git reset --hard upstream/master to clean up your master branch.

switham · 2014-10-18T06:05:44Z

#3662 is "Make all classes new-style."

pelson reviewed Oct 10, 2014
View reviewed changes

tacaswell reviewed Oct 10, 2014
View reviewed changes

switham added 2 commits October 14, 2014 20:09

Merge branch 'master' of https://github.com/matplotlib/matplotlib

7145d19

rm trailing whitespace added in 828bc58 or 0385071

1f07ab1

tacaswell added a commit that referenced this pull request Oct 16, 2014

Merge pull request #3628 from switham/master

7f71233

DOC : Document auto-init behavior of colors.Normalize and cm.ScalarMappable.

tacaswell merged commit 7f71233 into matplotlib:master Oct 16, 2014

switham mentioned this pull request Oct 18, 2014

Make all classes new-style. #3662

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document auto-init behavior of colors.Normalize and cm.ScalarMappable. #3628

Document auto-init behavior of colors.Normalize and cm.ScalarMappable. #3628

switham commented Oct 10, 2014

switham commented Oct 10, 2014

pelson Oct 10, 2014

switham Oct 14, 2014

pelson commented Oct 10, 2014

jenshnielsen commented Oct 10, 2014

jenshnielsen commented Oct 10, 2014

tacaswell Oct 10, 2014

switham Oct 14, 2014

switham commented Oct 15, 2014

jenshnielsen commented Oct 15, 2014

tacaswell commented Oct 15, 2014

switham commented Oct 15, 2014

tacaswell commented Oct 15, 2014

pelson commented Oct 15, 2014

switham commented Oct 16, 2014

tacaswell commented Oct 16, 2014

switham commented Oct 18, 2014

Document auto-init behavior of colors.Normalize and cm.ScalarMappable. #3628

Document auto-init behavior of colors.Normalize and cm.ScalarMappable. #3628

Conversation

switham commented Oct 10, 2014

switham commented Oct 10, 2014

pelson Oct 10, 2014

Choose a reason for hiding this comment

switham Oct 14, 2014

Choose a reason for hiding this comment

pelson commented Oct 10, 2014

jenshnielsen commented Oct 10, 2014

jenshnielsen commented Oct 10, 2014

tacaswell Oct 10, 2014

Choose a reason for hiding this comment

switham Oct 14, 2014

Choose a reason for hiding this comment

switham commented Oct 15, 2014

jenshnielsen commented Oct 15, 2014

tacaswell commented Oct 15, 2014

switham commented Oct 15, 2014

tacaswell commented Oct 15, 2014

pelson commented Oct 15, 2014

switham commented Oct 16, 2014

tacaswell commented Oct 16, 2014

switham commented Oct 18, 2014