Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Updated patch to not override alpha on edgecolor if set to none #1938

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

cimarronm commented Apr 23, 2013

Addresses issue #1934.

Member

pelson commented Apr 24, 2013

Hmmm. I'm not sure about this. This will result in some strange behaviour. The following two snippets will behave very differently:

# set the edgecolor to black, but make it completely transparent
>>> patch.set_edgecolor((0, 0, 0, 0))
>>> patch.set_alpha(0.5)
>>> print patch.get_edgecolor()
(0, 0, 0, 0)
# set the edgecolor to red, but make it completely transparent
>>> patch.set_edgecolor((1, 0, 0, 0))
>>> patch.set_alpha(0.5)
>>> print patch.get_edgecolor()
(1, 0, 0, 0.5)

It may be that the work stemming from #1899 should address this problem when dealing with alpha and color definitions.

Contributor

cimarronm commented Apr 26, 2013

@pelson: You are correct. The problem is that there is no special color designation for no edge. Instead 'none' gets translated in set_edgecolor into (0,0,0,0) and it becomes black with alpha=0 (i.e. one loses the context that it was no edge).

The real solution I see is no allow a color designation of none. But that is not straightforward with the current code that I can see. There are three posibilities I can readily see:

  1. Allow color to be a RGBA tuple or 'none' - code handling the color value gets a bit ugly as you have to check if it is a string or list/tuple.
  2. Allow color to be a RGBA tuple or None - much cleaner implementation but the problem is the existing setter/getter UI associates None with a default color rather than no color (it would have been much better to use 'default' or something like that instead of None for the setter IMHO but it is what it is).
  3. Allow color to be a RGBA tuple but use a special tuple value for a no-color designation, e.g., (-1,-1,-1,-1) - this seems like the best solution to me but not sure what your/others opinion on it

Do you think one of the three is the way to go or do you see an alternative approach?

Contributor

Westacular commented Apr 27, 2013

Actually, the way facecolor is handled in Patch served as a template for what edgecolor ought to do -- internally preserve what edgecolor was originally set to, so in cases like edgecolor='none', it knows to not let alpha override that (the logic for this is already present in colorConverter.to_rgba()). Commit 461233c in #1954 makes this change.

Contributor

cimarronm commented May 11, 2013

Sounds good to me. I will close this PR

@cimarronm cimarronm closed this May 11, 2013

Member

pelson commented May 13, 2013

Thanks to @cimarronm and @Westacular for looking into this murky part of color handling 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment