Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove _orig_color which is duplicate of _rgb #3683

Merged
merged 1 commit into from Nov 13, 2014

Conversation

jbmohler
Copy link
Contributor

The GraphicsContextBase._orig_color member was introduced by 6cc8aaa , but I don't believe it is necessary. This patch removes that member and goes a small step further to put related special case in set_foreground.

This is a spin-off of #3393 where test coverage is a legit concern. A default tests.py run on my linux box covers all paths touched by this code with the exception of set_greylevel.

@tacaswell tacaswell added this to the v1.5.x milestone Oct 20, 2014
@mdboom
Copy link
Member

mdboom commented Oct 20, 2014

@Westacular: Any thoughts?

@jbmohler
Copy link
Contributor Author

I was weighing writing a test that tests set_graylevel when I discovered that it doesn't appear to be called anywhere (aside from calling the base class in overrides). Since backends are not part of the surface area which anyone uses outside of the MPL library this leads me to believe that the set_graylevel method is now unnecessary.

Would like your feedback on this .. @mdboom @tacaswell . Thanks.

@tacaswell
Copy link
Member

Searching gitk it appears it has never been called except by subclasses passing up the inheritance chain...

There are user-defined backends out there in the wild so this will have to go through a deprecation cycle (document, at least one release cycle of raising a warning), but I'm in favor of getting rid of it.

@efiring
Copy link
Member

efiring commented Oct 21, 2014

Just a historical note: color handling has long been a source of difficulty and confusion, largely because of the way alpha handling has evolved, combined with the original drawing model in mpl. The fact that postscript doesn't have transparency at all makes things worse. In retrospect, I suspect the inclusion of an alpha kwarg at the artist level is probably a design error, and working around it has led to some of the convoluted code you are stumbling over.

Every now and then there has been a burst of effort to fix some of the problems, and usually that leads to the conclusion that a major revamp of color handling is in order; but obviously, none of us has gotten around to executing that.

tacaswell added a commit that referenced this pull request Nov 13, 2014
MNT : remove GraphicsContextBase._orig_color which is duplicate of _rgb

This is a hold over from the old alpha handling code that is now redundant as we carry rgba values through out.
@tacaswell tacaswell merged commit 5ffada3 into matplotlib:master Nov 13, 2014
@jbmohler jbmohler deleted the nix_orig_color branch November 17, 2014 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants