Bug in 'weights' in axes.hist

Hello,

The documentation for hist seems to indicate that you should be able
to send a list of values through the 'weights' parameter in axes.hist,
and this worked in previous versions. In 1.0, however, this produces
an error. I've attached a diff (also pasted below) that I believe
produces the expected behavior.

It can be tested with:
plt.hist([1,2,3], weights=[1,2,3])

The above fails in the development version, but works with the diff.
Could someone add this fix?

Thanks,
Jeff

Jeff Klukas, Research Assistant, Physics
University of Wisconsin -- Madison
jeff.klukas@...830... | jeffyklukas@...831... | jeffklukas@...832...
http://klukas.web.cern.ch/

Index: lib/matplotlib/axes.py

weights.diff (695 Bytes)

···

===================================================================
--- lib/matplotlib/axes.py (revision 8565)
+++ lib/matplotlib/axes.py (working copy)
@@ -7587,7 +7587,12 @@
                 else:
                     raise ValueError("weights must be 1D or 2D")
             else:
- w = [np.array(wi) for wi in weights]
+ try:
+ weights[0][0]
+ except TypeError:
+ w = [np.array(weights)]
+ else:
+ w = [np.array(wi) for wi in weights]

             if len(w) != nx:
                 raise ValueError('weights should have the same shape as x')

Good catch, Jeff. Looking over the code, looks like both the input data, x, and the weights get similar pre-processing done to ready it for histogramming. It appears that a fix was made to how x was being processed, but the same was not done to weights. I have a patch that fixes the pre-processing of weights, and also adds comments to both blocks of code to remind future developers to make sure changes are made to both chunks of code.

The functional part of the change was to check if the first element of weights was an iterable or not. Before, the weights array as in the given example would be considered 1-element weights for 3 datasets, rather than 3-element weights for 1 dataset.

Ben Root

histWeights.patch (1 KB)

···

On Tue, Jul 20, 2010 at 9:21 AM, Jeff Klukas <klukas@…45…> wrote:

Hello,

The documentation for hist seems to indicate that you should be able

to send a list of values through the ‘weights’ parameter in axes.hist,

and this worked in previous versions. In 1.0, however, this produces

an error. I’ve attached a diff (also pasted below) that I believe

produces the expected behavior.

It can be tested with:

plt.hist([1,2,3], weights=[1,2,3])

The above fails in the development version, but works with the diff.

Could someone add this fix?

Thanks,

Jeff

Jeff Klukas, Research Assistant, Physics

University of Wisconsin – Madison

jeff.klukas@…830… | jeffyklukas@…831… | jeffklukas@…832…

http://klukas.web.cern.ch/

Index: lib/matplotlib/axes.py

===================================================================

— lib/matplotlib/axes.py (revision 8565)

+++ lib/matplotlib/axes.py (working copy)

@@ -7587,7 +7587,12 @@

             else:

                 raise ValueError("weights must be 1D or 2D")

         else:
  •            w = [np.array(wi) for wi in weights]
    
  •            try:
    
  •                weights[0][0]
    
  •            except TypeError:
    
  •                w = [np.array(weights)]
    
  •            else:
    
  •                w = [np.array(wi) for wi in weights]
    
    
    
           if len(w) != nx:
    
               raise ValueError('weights should have the same shape as x')
    

Re-pinging on my proposed patch. Also, should it go into just the trunk, or should it also go into the branch?

Ben Root

···

On Sun, Jul 25, 2010 at 3:51 PM, Benjamin Root <ben.root@…553…> wrote:

On Tue, Jul 20, 2010 at 9:21 AM, Jeff Klukas <klukas@…45…> wrote:

Hello,

The documentation for hist seems to indicate that you should be able

to send a list of values through the ‘weights’ parameter in axes.hist,

and this worked in previous versions. In 1.0, however, this produces

an error. I’ve attached a diff (also pasted below) that I believe

produces the expected behavior.

It can be tested with:

plt.hist([1,2,3], weights=[1,2,3])

The above fails in the development version, but works with the diff.

Could someone add this fix?

Thanks,

Jeff

Jeff Klukas, Research Assistant, Physics

University of Wisconsin – Madison

jeff.klukas@…830… | jeffyklukas@…831… | jeffklukas@…832…

http://klukas.web.cern.ch/

Index: lib/matplotlib/axes.py

===================================================================

— lib/matplotlib/axes.py (revision 8565)

+++ lib/matplotlib/axes.py (working copy)

@@ -7587,7 +7587,12 @@

             else:

                 raise ValueError("weights must be 1D or 2D")

         else:
  •            w = [np.array(wi) for wi in weights]
    
  •            try:
    
  •                weights[0][0]
    
  •            except TypeError:
    
  •                w = [np.array(weights)]
    
  •            else:
    
  •                w = [np.array(wi) for wi in weights]
    
    
    
           if len(w) != nx:
    
               raise ValueError('weights should have the same shape as x')
    

Good catch, Jeff. Looking over the code, looks like both the input data, x, and the weights get similar pre-processing done to ready it for histogramming. It appears that a fix was made to how x was being processed, but the same was not done to weights. I have a patch that fixes the pre-processing of weights, and also adds comments to both blocks of code to remind future developers to make sure changes are made to both chunks of code.

The functional part of the change was to check if the first element of weights was an iterable or not. Before, the weights array as in the given example would be considered 1-element weights for 3 datasets, rather than 3-element weights for 1 dataset.

Ben Root

[...]

    Good catch, Jeff. Looking over the code, looks like both the input
    data, x, and the weights get similar pre-processing done to ready it
    for histogramming. It appears that a fix was made to how x was being
    processed, but the same was not done to weights. I have a patch
    that fixes the pre-processing of weights, and also adds comments to
    both blocks of code to remind future developers to make sure changes
    are made to both chunks of code.

    The functional part of the change was to check if the first element
    of weights was an iterable or not. Before, the weights array as in
    the given example would be considered 1-element weights for 3
    datasets, rather than 3-element weights for 1 dataset.

    Ben Root

Re-pinging on my proposed patch. Also, should it go into just the
trunk, or should it also go into the branch?

Ben,

Go ahead, trunk and branch, since it is a bugfix.

Thank you.

Eric

···

On 07/29/2010 09:31 AM, Benjamin Root wrote:

Ben Root

Done in r8595 and r8596.

As a side-note, it looks like various files that have been changed due to svnmerge.py are still showing themselves as having their properties modified. Is this ok?

Ben Root

···

On Thu, Jul 29, 2010 at 3:39 PM, Eric Firing <efiring@…229…> wrote:

On 07/29/2010 09:31 AM, Benjamin Root wrote:

[…]

Good catch, Jeff.  Looking over the code, looks like both the input
data, x, and the weights get similar pre-processing done to ready it
for histogramming. It appears that a fix was made to how x was being
processed, but the same was not done to weights.  I have a patch
that fixes the pre-processing of weights, and also  adds comments to
both blocks of code to remind future developers to make sure changes
are made to both chunks of code.
The functional part of the change was to check if the first element
of weights was an iterable or not.  Before, the weights array as in
the given example would be considered 1-element weights for 3
datasets, rather than 3-element weights for 1 dataset.
Ben Root

Re-pinging on my proposed patch. Also, should it go into just the

trunk, or should it also go into the branch?

Ben,

Go ahead, trunk and branch, since it is a bugfix.

Thank you.

Eric

    [...]
     >
     > Good catch, Jeff. Looking over the code, looks like both the
    input
     > data, x, and the weights get similar pre-processing done to
    ready it
     > for histogramming. It appears that a fix was made to how x
    was being
     > processed, but the same was not done to weights. I have a patch
     > that fixes the pre-processing of weights, and also adds
    comments to
     > both blocks of code to remind future developers to make sure
    changes
     > are made to both chunks of code.
     >
     > The functional part of the change was to check if the first
    element
     > of weights was an iterable or not. Before, the weights array
    as in
     > the given example would be considered 1-element weights for 3
     > datasets, rather than 3-element weights for 1 dataset.
     >
     > Ben Root
     >
     > Re-pinging on my proposed patch. Also, should it go into just the
     > trunk, or should it also go into the branch?

    Ben,

    Go ahead, trunk and branch, since it is a bugfix.

    Thank you.

    Eric

Done in r8595 and r8596.

As a side-note, it looks like various files that have been changed due
to svnmerge.py are still showing themselves as having their properties
modified. Is this ok?

That seems to be the way svnmerge works--it almost always generates an alarming list of property changes.

Eric

···

On 07/29/2010 01:17 PM, Benjamin Root wrote:

On Thu, Jul 29, 2010 at 3:39 PM, Eric Firing <efiring@…229… > <mailto:efiring@…229…>> wrote:
    On 07/29/2010 09:31 AM, Benjamin Root wrote:

Ben Root

Yes, for some reason some of the files, like axes3d examples, are
always tagged with property changes on svn merges. It's a nuisance.
but harmless. In the handling of weights vs x, I notice in some
places we call np.array and others np.asarray. Shouldn't we be using
asarray in all of these cases, eg

Index: lib/matplotlib/axes.py

···

On Thu, Jul 29, 2010 at 6:17 PM, Benjamin Root <ben.root@...553...> wrote:

As a side-note, it looks like various files that have been changed due to
svnmerge.py are still showing themselves as having their properties
modified. Is this ok?

===================================================================
--- lib/matplotlib/axes.py (revision 8606)
+++ lib/matplotlib/axes.py (working copy)
@@ -7401,11 +7401,13 @@
              **kwargs):
         """
         call signature::
+
+ def hist(x, bins=10, range=None, normed=False, weights=None,
+ cumulative=False, bottom=None, histtype='bar', align='mid',
+ orientation='vertical', rwidth=None, log=False,
+ color=None, label=None,
+ **kwargs):

- hist(x, bins=10, range=None, normed=False, cumulative=False,
- bottom=None, histtype='bar', align='mid',
- orientation='vertical', rwidth=None, log=False, **kwargs)
-
         Compute and draw the histogram of *x*. The return value is a
         tuple (*n*, *bins*, *patches*) or ([*n0*, *n1*, ...], *bins*,
         [*patches0*, *patches1*,...]) if the input contains multiple
@@ -7567,7 +7569,7 @@
                     'this looks transposed (shape is %d x %d)' % x.shape[::-1])
         else:
             # multiple hist with data of different length
- x = [np.array(xi) for xi in x]
+ x = [np.asarray(xi) for xi in x]

         nx = len(x) # number of datasets

@@ -7582,7 +7584,7 @@
         # We need to do to 'weights' what was done to 'x'
         if weights is not None:
             if isinstance(weights, np.ndarray) or not iterable(weights[0]) :
- w = np.array(weights)
+ w = np.asarray(weights)
                 if w.ndim == 2:
                     w = w.T
                 elif w.ndim == 1:
@@ -7590,7 +7592,7 @@
                 else:
                     raise ValueError("weights must be 1D or 2D")
             else:
- w = [np.array(wi) for wi in weights]
+ w = [np.asarray(wi) for wi in weights]

             if len(w) != nx:
                 raise ValueError('weights should have the same shape as x')