On 08/13/2010 10:35 AM, Benjamin Root wrote: > On Thu, Aug 12, 2010 at 4:46 PM, Eric Firing <efir...@hawaii.edu > <mailto:efir...@hawaii.edu>> 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. 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 > > 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 https://lists.sourceforge.net/lists/listinfo/matplotlib-devel