matplotlib-devel Mailing List for matplotlib

On Tue, Oct 16, 2012 at 6:36 PM, T J <tj...@gm...> wrote:
> There seems to be an issue with how arguments are parsed when it comes
> to determining the color of a line. Generally, it seems that 'c'
> takes precedence over 'color'. However, this precedence seems to
> change with the number of passed kwargs.
>
>
Yeah, there is a bit of a can of worms here. I suspect that part of the
issue has to do with dictionaries not guaranteeing order, but I can't
figure out where that would happen, though. In the plot handling code, we
are explicitly looking for the "color" kwarg, never the 'c' kwarg. It is
over in the Line2D class that the "c" kwarg ever gets obtained. In that
situation, what is likely happening is that the plot() code creates the
Line2D object, passing the "color" kwarg that it extracted, and any other
kwargs to the Line2D constructor (such as the 'c' kwarg). My guess is that
the whole aliasing system is rather ad-hoc, and actually doesn't have
explicit precedence because it was never intended for both kwargs to appear
at the same time.
Could you file an issue on the github tracker about this?
Thank you,
Ben Root

Thanks for your comments! see below...
On 10/16/2012 10:14 AM, Damon McDougall wrote:
> On Tue, Oct 16, 2012 at 2:55 PM, Kevin Davies <kda...@gm...> wrote:
>> Hello,
>>
>> I made a few minor changes to the Sankey class. They are listed at:
>> https://github.com/kdavies4/matplotlib/compare/master...sankey5
>>
>> Please review this and let me know if I can submit a pull request.
>>
>> Thanks.
>>
>> Kevin
> Thanks for taking the time to fix up a part of the codebase!
>
> If you make a pull request out of your code, we'll be able to leave
> inline comments on your patches. Nonetheless, I have some feedback for
> you after a (very) quick glance:
I submitted the pull request.
> 1) I don't think 'orientations' is a python keyword. What is the error
> you were getting? In any case, changing it breaks backwards
> compatibility so I'd be an advocate of keeping 'orientations'. Unless,
> of course, the error you were getting was serious.
The problem was on my end (In my code, I intercepted orientations as a 
named argument but assumed it being passed through **kwargs). I 
reverted to the original. Thanks for your patience. I'm sorry.
> 2) Your changes appear to be, mainly, cosmetic. This is good but may
> cause issues with some of the PEP8 pull requests we have been getting.
> Have you rebased to make sure these changes are incorporated?
I rebased off master after pulling from origin. That's correct, right?
> 3) Inline with my PEP8 remark in 2) above. You have some lines (maybe
> only one or two) that look longer than 79 characters.
I re-wrapped everything to 79 characters.
> Other than that, I think you should turn this into a pull request so
> you can get more feedback on an interactive level.
>
> Best,
> Damon
>