Handle 'none' as edgecolor for bar()

Development matplotlib-devel
1

Hi,

I tried to use "edgecolor = 'none'" in a call to bar(), hoping to get no
border to the bars, but instead got no bars at all. The patch below
(against 1.0.0) seems to fix this; it adds a check for 'none' to the
existing check for None as a special case of the edgecolor argument.

Thanks,

Ben.

--- ORIG-axes.py 2010-07-06 15:43:35.000000000 +0100
+++ NEW-axes.py 2010-08-09 09:16:51.000004000 +0100
@@ -4582,8 +4582,9 @@
             if len(color) < nbars:
                 color *= nbars

- if edgecolor is None:
- edgecolor = [None] * nbars
+ if (edgecolor is None
+ or (is_string_like(edgecolor) and edgecolor.lower() == 'none')):
+ edgecolor = [edgecolor] * nbars
         else:
             edgecolor = list(mcolors.colorConverter.to_rgba_array(edgecolor))
             if len(edgecolor) < nbars:

2

Just to note, the documentation does specify a difference between None and ‘none’. None means to use the rcdefaults and ‘none’ means no color at all. Is bar() just simply not properly handling the ‘none’ case?

Ben Root

···

On Mon, Aug 9, 2010 at 3:28 AM, Ben North <ben@…544…> wrote:

Hi,

I tried to use “edgecolor = ‘none’” in a call to bar(), hoping to get no

border to the bars, but instead got no bars at all. The patch below

(against 1.0.0) seems to fix this; it adds a check for ‘none’ to the

existing check for None as a special case of the edgecolor argument.

Thanks,

Ben.

— ORIG-axes.py 2010-07-06 15:43:35.000000000 +0100

+++ NEW-axes.py 2010-08-09 09:16:51.000004000 +0100

@@ -4582,8 +4582,9 @@

         if len(color) < nbars:

             color *= nbars

  •    if edgecolor is None:

  •        edgecolor = [None] * nbars

  •    if (edgecolor is None

  •        or (is_string_like(edgecolor) and edgecolor.lower() == 'none')):

  •        edgecolor = [edgecolor] * nbars

   else:

       edgecolor = list(mcolors.colorConverter.to_rgba_array(edgecolor))

       if len(edgecolor) < nbars:
3

I tried to use "edgecolor = 'none'" in a call to bar(), hoping to get no
border to the bars, but instead got no bars at all.

Just to note, the documentation does specify a difference between None and
'none'. None means to use the rcdefaults and 'none' means no color at all.
Is bar() just simply not properly handling the 'none' case?

That's right, yes. Currently, bar() does not handle the string 'none'
properly; it results in an empty graph. E.g.:

   bar([1, 2, 3], [12, 13, 14], edgecolor = None)

behaves correctly, giving a bar chart with black-edged blue bars.

   bar([1, 2, 3], [12, 13, 14], edgecolor = 'none')

gives no graph at all. After the patch, the second call gives the right
result, a bar-chart with border-less blue bars. Same kind of thing with
the kwarg 'color' instead of 'edgecolor', which is also fixed in my
second recent email.

Hope this clarifies things. Thanks,

Ben.

4

Looking through the code for bar(), I see the same thing occurs for the ‘color’ keyword argument. So I guess we should fix that as well. If no one has any objections, I can commit this fix.

Ben Root

···

On Mon, Aug 9, 2010 at 10:02 AM, Ben North <ben@…544…> wrote:

I tried to use “edgecolor = ‘none’” in a call to bar(), hoping to get no

border to the bars, but instead got no bars at all.

Just to note, the documentation does specify a difference between None and

‘none’. None means to use the rcdefaults and ‘none’ means no color at all.

Is bar() just simply not properly handling the ‘none’ case?

That’s right, yes. Currently, bar() does not handle the string ‘none’

properly; it results in an empty graph. E.g.:

bar([1, 2, 3], [12, 13, 14], edgecolor = None)

behaves correctly, giving a bar chart with black-edged blue bars.

bar([1, 2, 3], [12, 13, 14], edgecolor = ‘none’)

gives no graph at all. After the patch, the second call gives the right

result, a bar-chart with border-less blue bars. Same kind of thing with

the kwarg ‘color’ instead of ‘edgecolor’, which is also fixed in my

second recent email.

Hope this clarifies things. Thanks,

Ben.

5

Ben Root:

Ben North:

Same kind of thing with
the kwarg 'color' instead of 'edgecolor', which is also fixed in my
second recent email.

Looking through the code for bar(), I see the same thing occurs for the
'color' keyword argument. So I guess we should fix that as well.

Yes, the second of the emails I sent (pasted below) fixes the behaviour
for 'edgecolor' and 'color'. Thanks for committing this.

Ben.

···

------------------------------------------------------------------------

Date: 9 August 2010 09:42
Subject: Handle 'none' as color and edgecolor for bar()

Hi,

Update to my recent email: perhaps it would make sense to handle the
'color' argument in the same way, allowing hollow bars. Combined patch
below.

Ben.

--- ORIG-axes.py 2010-07-06 15:43:35.000000000 +0100
+++ NEW-axes.py 2010-08-09 09:39:44.000256000 +0100
@@ -4575,15 +4575,17 @@
        if len(linewidth) < nbars:
            linewidth *= nbars

- if color is None:
- color = [None] * nbars
+ if (color is None
+ or (is_string_like(color) and color.lower() == 'none')):
+ color = [color] * nbars
        else:
            color = list(mcolors.colorConverter.to_rgba_array(color))
            if len(color) < nbars:
                color *= nbars

- if edgecolor is None:
- edgecolor = [None] * nbars
+ if (edgecolor is None
+ or (is_string_like(edgecolor) and edgecolor.lower() == 'none')):
+ edgecolor = [edgecolor] * nbars
        else:
            edgecolor = list(mcolors.colorConverter.to_rgba_array(edgecolor))
            if len(edgecolor) < nbars:

6

Ben Root:

Ben North:

Same kind of thing with

the kwarg ‘color’ instead of ‘edgecolor’, which is also fixed in my

second recent email.

Looking through the code for bar(), I see the same thing occurs for the

‘color’ keyword argument. So I guess we should fix that as well.

Yes, the second of the emails I sent (pasted below) fixes the behaviour

for ‘edgecolor’ and ‘color’. Thanks for committing this.

Ben.

Date: 9 August 2010 09:42

Subject: Handle ‘none’ as color and edgecolor for bar()

Hi,

Update to my recent email: perhaps it would make sense to handle the

‘color’ argument in the same way, allowing hollow bars. Combined patch

below.

Ben.

— ORIG-axes.py 2010-07-06 15:43:35.000000000 +0100

+++ NEW-axes.py 2010-08-09 09:39:44.000256000 +0100

@@ -4575,15 +4575,17 @@

    if len(linewidth) < nbars:

        linewidth *= nbars

  •    if color is None:

  •        color = [None] * nbars

  •    if (color is None

  •        or (is_string_like(color) and color.lower() == 'none')):

  •        color = [color] * nbars

  else:

      color = list(mcolors.colorConverter.to_rgba_array(color))

if len(color) < nbars:

            color *= nbars

  •    if edgecolor is None:

  •        edgecolor = [None] * nbars

  •    if (edgecolor is None

  •        or (is_string_like(edgecolor) and edgecolor.lower() == 'none')):

  •        edgecolor = [edgecolor] * nbars

  else:

      edgecolor = list(mcolors.colorConverter.to_rgba_array(edgecolor))

      if len(edgecolor) < nbars:

I did a little more checking to see if this was needed elsewhere and I think this is the wrong patch to apply. It appears that colorConverter.to_rgba_array() might not be correctly handling the case of ‘none’. If one passes in a list of colors with some of them being ‘none’, everything works nicely. However, if one passes in just ‘none’, then it returns an empty array. Note that passing in [‘none’] to .to_rgba_array() produces a length 1 array.

mcolor.colorConvertor.to_rgba_array(‘none’)
array(, shape=(0, 4), dtype=float64)

mcolor.colorConvertor.to_rgba_array([‘none’])
array([[ 0., 0., 0., 0.]])

mcolor.colorConvertor.to_rgba_array(‘r’)
array([[ 1., 0., 0., 1.]])

Should this be regarded as a bug?

Ben Root

···

On Thu, Aug 12, 2010 at 10:13 AM, Ben North <ben@…544…> wrote:

7

    Ben Root:
     >Ben North:
     >> Same kind of thing with
     >> the kwarg 'color' instead of 'edgecolor', which is also fixed in my
     >> second recent email.
     >
     > Looking through the code for bar(), I see the same thing occurs
    for the
     > 'color' keyword argument. So I guess we should fix that as well.

    Yes, the second of the emails I sent (pasted below) fixes the behaviour
    for 'edgecolor' and 'color'. Thanks for committing this.

    Ben.

    ------------------------------------------------------------------------

    Date: 9 August 2010 09:42
    Subject: Handle 'none' as color and edgecolor for bar()

    Hi,

    Update to my recent email: perhaps it would make sense to handle the
    'color' argument in the same way, allowing hollow bars. Combined patch
    below.

    Ben.

    --- ORIG-axes.py 2010-07-06 15:43:35.000000000 +0100
    +++ NEW-axes.py 2010-08-09 09:39:44.000256000 +0100
    @@ -4575,15 +4575,17 @@
            if len(linewidth) < nbars:
                linewidth *= nbars

    - if color is None:
    - color = [None] * nbars
    + if (color is None
    + or (is_string_like(color) and color.lower() == 'none')):
    + color = [color] * nbars
            else:
                color = list(mcolors.colorConverter.to_rgba_array(color))
                if len(color) < nbars:
                    color *= nbars

    - if edgecolor is None:
    - edgecolor = [None] * nbars
    + if (edgecolor is None
    + or (is_string_like(edgecolor) and edgecolor.lower() ==
    'none')):
    + edgecolor = [edgecolor] * nbars
            else:
                edgecolor =
    list(mcolors.colorConverter.to_rgba_array(edgecolor))
                if len(edgecolor) < nbars:

I did a little more checking to see if this was needed elsewhere and I
think this is the wrong patch to apply. It appears that
colorConverter.to_rgba_array() might not be correctly handling the case
of 'none'. If one passes in a list of colors with some of them being
'none', everything works nicely. However, if one passes in just 'none',
then it returns an empty array. Note that passing in ['none'] to
.to_rgba_array() produces a length 1 array.

>>> mcolor.colorConvertor.to_rgba_array('none')
array(, shape=(0, 4), dtype=float64)

>>> mcolor.colorConvertor.to_rgba_array(['none'])
array([[ 0., 0., 0., 0.]])

>>> mcolor.colorConvertor.to_rgba_array('r')
array([[ 1., 0., 0., 1.]])

Should this be regarded as a bug?

Yes, 'none' should be handled uniformly by that method. Thanks for tracking down actual source of the problem. Fixing it there is the right thing to do.

Eric

···

On 08/12/2010 06:09 AM, Benjamin Root wrote:

On Thu, Aug 12, 2010 at 10:13 AM, Ben North <ben@...544... > <mailto:ben@…544…>> wrote:

Ben Root

------------------------------------------------------------------------------
This SF.net email is sponsored by

Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev

_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
matplotlib-devel List Signup and Options

8

I am assuming that we would like this patched in the maintenance branch, too, right? Also, because the doc and the output of the .to_rgba_array() function is changing, should it be noted in the changelog?

Ben Root

···

On Thu, Aug 12, 2010 at 12:37 PM, Eric Firing <efiring@…229…> wrote:

On 08/12/2010 06:09 AM, Benjamin Root wrote:

On Thu, Aug 12, 2010 at 10:13 AM, Ben North <ben@…544… > > mailto:ben@...544...> wrote:

Ben Root:

 >Ben North:

 >> Same kind of thing with

 >> the kwarg 'color' instead of 'edgecolor', which is also fixed in my

 >> second recent email.

 >

 > Looking through the code for bar(), I see the same thing occurs

for the

 > 'color' keyword argument.  So I guess we should fix that as well.

Yes, the second of the emails I sent (pasted below) fixes the behaviour

for 'edgecolor' and 'color'.  Thanks for committing this.

Ben.

------------------------------------------------------------------------

Date: 9 August 2010 09:42

Subject: Handle 'none' as color and edgecolor for bar()

Hi,

Update to my recent email: perhaps it would make sense to handle the

'color' argument in the same way, allowing hollow bars.  Combined patch

below.

Ben.

--- ORIG-axes.py        2010-07-06 15:43:35.000000000 +0100

+++ NEW-axes.py 2010-08-09 09:39:44.000256000 +0100

@@ -4575,15 +4575,17 @@

        if len(linewidth) < nbars:

            linewidth *= nbars

-        if color is None:

-            color = [None] * nbars

+        if (color is None

+            or (is_string_like(color) and color.lower() == 'none')):

+            color = [color] * nbars

        else:

            color = list(mcolors.colorConverter.to_rgba_array(color))

            if len(color) < nbars:

                color *= nbars

-        if edgecolor is None:

-            edgecolor = [None] * nbars

+        if (edgecolor is None

+            or (is_string_like(edgecolor) and edgecolor.lower() ==

'none')):

+            edgecolor = [edgecolor] * nbars

        else:

            edgecolor =

list(mcolors.colorConverter.to_rgba_array(edgecolor))

            if len(edgecolor) < nbars:

I did a little more checking to see if this was needed elsewhere and I

think this is the wrong patch to apply. It appears that

colorConverter.to_rgba_array() might not be correctly handling the case

of ‘none’. If one passes in a list of colors with some of them being

‘none’, everything works nicely. However, if one passes in just ‘none’,

then it returns an empty array. Note that passing in [‘none’] to

.to_rgba_array() produces a length 1 array.

mcolor.colorConvertor.to_rgba_array(‘none’)

array(, shape=(0, 4), dtype=float64)

mcolor.colorConvertor.to_rgba_array([‘none’])

array([[ 0., 0., 0., 0.]])

mcolor.colorConvertor.to_rgba_array(‘r’)

array([[ 1., 0., 0., 1.]])

Should this be regarded as a bug?

Yes, ‘none’ should be handled uniformly by that method. Thanks for

tracking down actual source of the problem. Fixing it there is the

right thing to do.

Eric

9

[...]

     >
     > >>> mcolor.colorConvertor.to_rgba_array('none')
     > array(, shape=(0, 4), dtype=float64)
     >
     > >>> mcolor.colorConvertor.to_rgba_array(['none'])
     > array([[ 0., 0., 0., 0.]])
     >
     > >>> mcolor.colorConvertor.to_rgba_array('r')
     > array([[ 1., 0., 0., 1.]])
     >
     > Should this be regarded as a bug?

    Yes, 'none' should be handled uniformly by that method. Thanks for
    tracking down actual source of the problem. Fixing it there is the
    right thing to do.

    Eric

I am assuming that we would like this patched in the maintenance branch,
too, right? Also, because the doc and the output of the
.to_rgba_array() function is changing, should it be noted in the changelog?

Yes, bugs should be squashed first in the maintenance branch, and svnmerge should be used to propagate the change to the trunk. If to_rgba_array is not treating "none" and ["none"] the same way, that is a bug.

But... now I'm looking at the to_rgba_array method, and wondering why it is specifying that special case handling of "none". The present code implementing that special case is mine, but I suspect I was just maintaining legacy behavior, as Darren had added this special case explicitly to the docstring long before my code change.

So it is looking more complicated than I thought. I suppose the course of action most consistent with the idea of a maintenance branch and a trunk would be to put the change in the trunk, since it is changing the documented behavior of a key method. Then the choices for the maintenance branch would be to work around the behavior, as in Ben North's patch, or to do nothing. If you work around it, I think it will require special attention to keep svnmerge from erroneously adding the workaround to the trunk the next time svnmerge is run. So, if you choose to do that at all, I would suggest waiting until you are sure how to handle that svnmerge aspect; maybe it is documented.

Also, with the change to to_rgba_array in the trunk, you will need to do some exploration to figure out whether any other code will need to be changed to take advantage of it, or to allow for it. (I may have had a reason for maintaining the bizarre legacy behavior the last time I changed the code in that method...)

Eric

···

On 08/12/2010 10:40 AM, Benjamin Root wrote:

Ben Root

10

I have dug further about this. I have found that the hist() function, as well as the bar family of functions are impacted by this issue. However, for hist(), if you try passing in ‘none’ for color in the old version, it errors out saying that it needs some color info. With this corrected version, it doesn’t error, but there are no lines drawn as well (I have to see if that is another bug).

The other place where I can see how this fix might cause issues is with regards to Collections and the classes that derive from that.

While I certainly think that the current behavior of to_rgba_array() is wrong, I am starting to get hesitant about changing this because there might be some sort of fundamental difference between how the backends are treating “array(, shape=(0, 4), dtype=float64)” and “array([0., 0., 0., 0.])”. The first is really easy to use as a “don’t draw anything” whereas the latter isn’t that obvious to the backends.

A particular case where this might cause trouble is for graphics formats that do not support transparencies. Because “array([0., 0., 0., 0.])” is fully transparent black, in formats like eps with a non-black background, the objects with this color will appear – although, it is already possible to do that with bar(…, color=[‘none’]).

What I think we have here is a need for a consistent way to indicate “I am never to be drawn” that fits in with the current paradigm of the rgba arrays. Maybe nan in the alpha channel?

Ben Root

···

On Thu, Aug 12, 2010 at 4:46 PM, Eric Firing <efiring@…229…> wrote:

On 08/12/2010 10:40 AM, Benjamin Root wrote:

[…]

 >

 > >>> mcolor.colorConvertor.to_rgba_array('none')

 > array([], shape=(0, 4), dtype=float64)

 >

 > >>> mcolor.colorConvertor.to_rgba_array(['none'])

 > array([[ 0.,  0.,  0.,  0.]])

 >

 > >>> mcolor.colorConvertor.to_rgba_array('r')

 > array([[ 1.,  0.,  0.,  1.]])

 >

 > Should this be regarded as a bug?

Yes, 'none' should be handled uniformly by that method.  Thanks for

tracking down actual source of the problem.  Fixing it there is the

right thing to do.

Eric

I am assuming that we would like this patched in the maintenance branch,

too, right? Also, because the doc and the output of the

.to_rgba_array() function is changing, should it be noted in the changelog?

Yes, bugs should be squashed first in the maintenance branch, and

svnmerge should be used to propagate the change to the trunk. If

to_rgba_array is not treating “none” and [“none”] the same way, that is

a bug.

But… now I’m looking at the to_rgba_array method, and wondering why it

is specifying that special case handling of “none”. The present code

implementing that special case is mine, but I suspect I was just

maintaining legacy behavior, as Darren had added this special case

explicitly to the docstring long before my code change.
So it is looking more complicated than I thought. I suppose the course

of action most consistent with the idea of a maintenance branch and a

trunk would be to put the change in the trunk, since it is changing the

documented behavior of a key method. Then the choices for the

maintenance branch would be to work around the behavior, as in Ben

North’s patch, or to do nothing. If you work around it, I think it will

require special attention to keep svnmerge from erroneously adding the

workaround to the trunk the next time svnmerge is run. So, if you

choose to do that at all, I would suggest waiting until you are sure how

to handle that svnmerge aspect; maybe it is documented.

Also, with the change to to_rgba_array in the trunk, you will need to do

some exploration to figure out whether any other code will need to be

changed to take advantage of it, or to allow for it. (I may have had a

reason for maintaining the bizarre legacy behavior the last time I

changed the code in that method…)

Eric

11

    [...]
     > >
     > > >>> mcolor.colorConvertor.to_rgba_array('none')
     > > array(, shape=(0, 4), dtype=float64)
     > >
     > > >>> mcolor.colorConvertor.to_rgba_array(['none'])
     > > array([[ 0., 0., 0., 0.]])
     > >
     > > >>> mcolor.colorConvertor.to_rgba_array('r')
     > > array([[ 1., 0., 0., 1.]])
     > >
     > > Should this be regarded as a bug?
     >
     > Yes, 'none' should be handled uniformly by that method.
      Thanks for
     > tracking down actual source of the problem. Fixing it there
    is the
     > right thing to do.
     >
     > Eric
     >
     > I am assuming that we would like this patched in the maintenance
    branch,
     > too, right? Also, because the doc and the output of the
     > .to_rgba_array() function is changing, should it be noted in the
    changelog?

    Yes, bugs should be squashed first in the maintenance branch, and
    svnmerge should be used to propagate the change to the trunk. If
    to_rgba_array is not treating "none" and ["none"] the same way, that is
    a bug.

    But... now I'm looking at the to_rgba_array method, and wondering why it
    is specifying that special case handling of "none". The present code
    implementing that special case is mine, but I suspect I was just
    maintaining legacy behavior, as Darren had added this special case
    explicitly to the docstring long before my code change.

    So it is looking more complicated than I thought. I suppose the course
    of action most consistent with the idea of a maintenance branch and a
    trunk would be to put the change in the trunk, since it is changing the
    documented behavior of a key method. Then the choices for the
    maintenance branch would be to work around the behavior, as in Ben
    North's patch, or to do nothing. If you work around it, I think it will
    require special attention to keep svnmerge from erroneously adding the
    workaround to the trunk the next time svnmerge is run. So, if you
    choose to do that at all, I would suggest waiting until you are sure how
    to handle that svnmerge aspect; maybe it is documented.

    Also, with the change to to_rgba_array in the trunk, you will need to do
    some exploration to figure out whether any other code will need to be
    changed to take advantage of it, or to allow for it. (I may have had a
    reason for maintaining the bizarre legacy behavior the last time I
    changed the code in that method...)

    Eric

I have dug further about this. I have found that the hist() function,
as well as the bar family of functions are impacted by this issue.
However, for hist(), if you try passing in 'none' for color in the old
version, it errors out saying that it needs some color info. With this
corrected version, it doesn't error, but there are no lines drawn as
well (I have to see if that is another bug).

The other place where I can see how this fix might cause issues is with
regards to Collections and the classes that derive from that.

While I certainly think that the current behavior of to_rgba_array() is
wrong, I am starting to get hesitant about changing this because there
might be some sort of fundamental difference between how the backends
are treating "array(, shape=(0, 4), dtype=float64)" and "array([0.,
0., 0., 0.])". The first is really easy to use as a "don't draw
anything" whereas the latter isn't that obvious to the backends.

A particular case where this might cause trouble is for graphics formats
that do not support transparencies. Because "array([0., 0., 0., 0.])"
is fully transparent black, in formats like eps with a non-black
background, the objects with this color will appear -- although, it is
already possible to do that with bar(..., color=['none']).

But the ps backend intercepts the 0-alpha value and interprets it as "don't draw at all". See the _draw_ps method:

         mightstroke = (gc.get_linewidth() > 0.0 and
                   (len(gc.get_rgb()) <= 3 or gc.get_rgb()[3] != 0.0))
         stroke = stroke and mightstroke
         fill = (fill and rgbFace is not None and
                 (len(rgbFace) <= 3 or rgbFace[3] != 0.0))

Notice that both stroke and fill are checking for alpha != 0.0.

All other major backends support alpha explicitly.

What I think we have here is a need for a consistent way to indicate "I
am never to be drawn" that fits in with the current paradigm of the rgba
arrays. Maybe nan in the alpha channel?

I think (95% confidence) that we have already settled on 0 in the alpha channel for that, (also zero-linewidth for lines should uniformly block drawing of a line) but perhaps had not done so the last time I worked on the to_rgba_array method. But I agree that this needs to be checked in the backends, and collections also need to be checked for side-effects of the change. My guess is it will be OK to make the change to to_rgba_array in the trunk. It can be checked fairly well via backend_driver.py.

Eric

···

On 08/13/2010 10:35 AM, Benjamin Root wrote:

On Thu, Aug 12, 2010 at 4:46 PM, Eric Firing <efiring@...229... > <mailto:efiring@…229…>> wrote:
    On 08/12/2010 10:40 AM, Benjamin Root wrote:

Ben Root

12

Yeah, well, try out my attached script. Then view the two files. Something is wrong…

Ben Root

nocolor_check.py (198 Bytes)

···

On Fri, Aug 13, 2010 at 4:43 PM, Eric Firing <efiring@…552…229…> wrote:

On 08/13/2010 10:35 AM, Benjamin Root wrote:

On Thu, Aug 12, 2010 at 4:46 PM, Eric Firing <efiring@…229… > > mailto:efiring@...229...> wrote:

On 08/12/2010 10:40 AM, Benjamin Root wrote:

[...]

 > >

 > > >>> mcolor.colorConvertor.to_rgba_array('none')

 > > array([], shape=(0, 4), dtype=float64)

 > >

 > > >>> mcolor.colorConvertor.to_rgba_array(['none'])

 > > array([[ 0.,  0.,  0.,  0.]])

 > >

 > > >>> mcolor.colorConvertor.to_rgba_array('r')

 > > array([[ 1.,  0.,  0.,  1.]])

 > >

 > > Should this be regarded as a bug?

 >

 >     Yes, 'none' should be handled uniformly by that method.

  Thanks for

 >     tracking down actual source of the problem.  Fixing it there

is the

 >     right thing to do.

 >

 >     Eric

 >

 >

 > I am assuming that we would like this patched in the maintenance

branch,

 > too, right?  Also, because the doc and the output of the

 > .to_rgba_array() function is changing, should it be noted in the

changelog?

Yes, bugs should be squashed first in the maintenance branch, and

svnmerge should be used to propagate the change to the trunk.  If

to_rgba_array is not treating "none" and ["none"] the same way, that is

a bug.

But... now I'm looking at the to_rgba_array method, and wondering why it

is specifying that special case handling of "none".  The present code

implementing that special case is mine, but I suspect I was just

maintaining legacy behavior, as Darren had added this special case

explicitly to the docstring long before my code change.

So it is looking more complicated than I thought.  I suppose the course

of action most consistent with the idea of a maintenance branch and a

trunk would be to put the change in the trunk, since it is changing the

documented behavior of a key method. Then the choices for the

maintenance branch would be to work around the behavior, as in Ben

North's patch, or to do nothing.  If you work around it, I think it will

require special attention to keep svnmerge from erroneously adding the

workaround to the trunk the next time svnmerge is run.  So, if you

choose to do that at all, I would suggest waiting until you are sure how

to handle that svnmerge aspect; maybe it is documented.

Also, with the change to to_rgba_array in the trunk, you will need to do

some exploration to figure out whether any other code will need to be

changed to take advantage of it, or to allow for it.  (I may have had a

reason for maintaining the bizarre legacy behavior the last time I

changed the code in that method...)

Eric

I have dug further about this. I have found that the hist() function,

as well as the bar family of functions are impacted by this issue.

However, for hist(), if you try passing in ‘none’ for color in the old

version, it errors out saying that it needs some color info. With this

corrected version, it doesn’t error, but there are no lines drawn as

well (I have to see if that is another bug).

The other place where I can see how this fix might cause issues is with

regards to Collections and the classes that derive from that.

While I certainly think that the current behavior of to_rgba_array() is

wrong, I am starting to get hesitant about changing this because there

might be some sort of fundamental difference between how the backends

are treating “array(, shape=(0, 4), dtype=float64)” and "array([0.,

0., 0., 0.])". The first is really easy to use as a "don’t draw

anything" whereas the latter isn’t that obvious to the backends.

A particular case where this might cause trouble is for graphics formats

that do not support transparencies. Because “array([0., 0., 0., 0.])”

is fully transparent black, in formats like eps with a non-black

background, the objects with this color will appear – although, it is

already possible to do that with bar(…, color=[‘none’]).

But the ps backend intercepts the 0-alpha value and interprets it as

“don’t draw at all”. See the _draw_ps method:

     mightstroke = (gc.get_linewidth() > 0.0 and

               (len(gc.get_rgb()) <= 3 or gc.get_rgb()[3] != 0.0))

     stroke = stroke and mightstroke

     fill = (fill and rgbFace is not None and

             (len(rgbFace) <= 3 or rgbFace[3] != 0.0))

Notice that both stroke and fill are checking for alpha != 0.0.

13

     >
     > [...]
     > > >
     > > > >>> mcolor.colorConvertor.to_rgba_array('none')
     > > > array(, shape=(0, 4), dtype=float64)
     > > >
     > > > >>> mcolor.colorConvertor.to_rgba_array(['none'])
     > > > array([[ 0., 0., 0., 0.]])
     > > >
     > > > >>> mcolor.colorConvertor.to_rgba_array('r')
     > > > array([[ 1., 0., 0., 1.]])
     > > >
     > > > Should this be regarded as a bug?
     > >
     > > Yes, 'none' should be handled uniformly by that method.
     > Thanks for
     > > tracking down actual source of the problem. Fixing it there
     > is the
     > > right thing to do.
     > >
     > > Eric
     > >
     > > I am assuming that we would like this patched in the maintenance
     > branch,
     > > too, right? Also, because the doc and the output of the
     > > .to_rgba_array() function is changing, should it be noted in the
     > changelog?
     >
     > Yes, bugs should be squashed first in the maintenance branch, and
     > svnmerge should be used to propagate the change to the trunk. If
     > to_rgba_array is not treating "none" and ["none"] the same
    way, that is
     > a bug.
     >
     > But... now I'm looking at the to_rgba_array method, and
    wondering why it
     > is specifying that special case handling of "none". The
    present code
     > implementing that special case is mine, but I suspect I was just
     > maintaining legacy behavior, as Darren had added this special
    case
     > explicitly to the docstring long before my code change.
     >
     > So it is looking more complicated than I thought. I suppose
    the course
     > of action most consistent with the idea of a maintenance
    branch and a
     > trunk would be to put the change in the trunk, since it is
    changing the
     > documented behavior of a key method. Then the choices for the
     > maintenance branch would be to work around the behavior, as
    in Ben
     > North's patch, or to do nothing. If you work around it, I
    think it will
     > require special attention to keep svnmerge from erroneously
    adding the
     > workaround to the trunk the next time svnmerge is run. So,
    if you
     > choose to do that at all, I would suggest waiting until you
    are sure how
     > to handle that svnmerge aspect; maybe it is documented.
     >
     > Also, with the change to to_rgba_array in the trunk, you will
    need to do
     > some exploration to figure out whether any other code will
    need to be
     > changed to take advantage of it, or to allow for it. (I may
    have had a
     > reason for maintaining the bizarre legacy behavior the last
    time I
     > changed the code in that method...)
     >
     > Eric
     >
     > I have dug further about this. I have found that the hist()
    function,
     > as well as the bar family of functions are impacted by this issue.
     > However, for hist(), if you try passing in 'none' for color in
    the old
     > version, it errors out saying that it needs some color info.
      With this
     > corrected version, it doesn't error, but there are no lines drawn as
     > well (I have to see if that is another bug).
     >
     > The other place where I can see how this fix might cause issues
    is with
     > regards to Collections and the classes that derive from that.
     >
     > While I certainly think that the current behavior of
    to_rgba_array() is
     > wrong, I am starting to get hesitant about changing this because
    there
     > might be some sort of fundamental difference between how the backends
     > are treating "array(, shape=(0, 4), dtype=float64)" and "array([0.,
     > 0., 0., 0.])". The first is really easy to use as a "don't draw
     > anything" whereas the latter isn't that obvious to the backends.
     >
     > A particular case where this might cause trouble is for graphics
    formats
     > that do not support transparencies. Because "array([0., 0., 0.,
    0.])"
     > is fully transparent black, in formats like eps with a non-black
     > background, the objects with this color will appear -- although,
    it is
     > already possible to do that with bar(..., color=['none']).

    But the ps backend intercepts the 0-alpha value and interprets it as
    "don't draw at all". See the _draw_ps method:

             mightstroke = (gc.get_linewidth() > 0.0 and
                       (len(gc.get_rgb()) <= 3 or gc.get_rgb()[3] != 0.0))
             stroke = stroke and mightstroke
             fill = (fill and rgbFace is not None and
                     (len(rgbFace) <= 3 or rgbFace[3] != 0.0))

    Notice that both stroke and fill are checking for alpha != 0.0.

Yeah, well, try out my attached script. Then view the two files.
Something is wrong...

Correct--good catch. It looks I missed the patch draw method when I was reworking the handling of alpha. Let me try to straighten that out in the next day or two.

Eric

···

On 08/13/2010 12:30 PM, Benjamin Root wrote:

On Fri, Aug 13, 2010 at 4:43 PM, Eric Firing <efiring@...229... > <mailto:efiring@…229…>> wrote:
    On 08/13/2010 10:35 AM, Benjamin Root wrote:
     > On Thu, Aug 12, 2010 at 4:46 PM, Eric Firing <efiring@...229... > <mailto:efiring@…229…> > > <mailto:efiring@…229…>> wrote:
     > On 08/12/2010 10:40 AM, Benjamin Root wrote:

Ben Root

14

[...]

    Notice that both stroke and fill are checking for alpha != 0.0.

Yeah, well, try out my attached script. Then view the two files.
Something is wrong...

I think I have that fixed now, along with a temporary fix for the original "none" problem. The better fix for the latter, involving to_rgb_array, can be done in the trunk.

The handling of alpha, and colors in general, is still confusing, providing abundant hiding places for bugs and opportunities for inefficiency. I hope that over time we can clarify and codify it.

For example, in some cases in the backends, the alpha value of an rgba does serve as a flag; in other cases, it is simply ignored, and the flag value is python None.

As far as I can see, at least for a Patch, one can explicitly set different alpha for the outline and for the face, but the backends can't render them that way. I don't see any reason why we can't fix that, but it would take some backend work. It might be fairly simple to factor it out into a pre_draw_path() that would call the backend separately for the edge and for the face if they have different alpha values, but could use the present single call otherwise. I suspect this could simplify the backends, relieving them of some of the checking they are doing now.

Eric

···

On 08/13/2010 12:30 PM, Benjamin Root wrote:

Ben Root