Skip to content

Using to_rgba rather than to_rgb in figure_edit #6589

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

Closed
wants to merge 1 commit into from

Conversation

dwgoon
Copy link

@dwgoon dwgoon commented Jun 15, 2016

OS: Windows 7 (64bit)
Python: Anaconda, Python 3.4.4 (64bit)
matplotlib: 1.5.1 (version used for this test)
matplotlib backend: Qt4Agg or Qt5Agg

If I set "markerfacecolor" as "none" (not None) for plotting, I get the following exception when I push the editing button of toolbar.

Traceback (most recent call last):
File "C:\Users\dwlee\Anaconda\envs\py34\lib\site-packages\matplotlib\colors.py", line 309, in to_rgb
fl = float(argl)
ValueError: could not convert string to float: 'none'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "C:\Users\dwlee\Anaconda\envs\py34\lib\site-packages\matplotlib\backends\backend_qt5.py", line 645, in edit_parameters
figureoptions.figure_edit(axes, self)
File "C:\Users\dwlee\Anaconda\envs\py34\lib\site-packages\matplotlib\backends\qt_editor\figureoptions.py", line 89, in
figure_edit
fc = rgb2hex(colorConverter.to_rgb(line.get_markerfacecolor()))
File "C:\Users\dwlee\Anaconda\envs\py34\lib\site-packages\matplotlib\colors.py", line 331, in to_rgb
'to_rgb: Invalid rgb arg "%s"\n%s' % (str(arg), exc))
ValueError: to_rgb: Invalid rgb arg "none" could not convert string to float: 'none'

Here is the example code for testing.

import numpy as np
import matplotlib.pyplot as plt

X = np.random.rand(100, 1000)
xs = np.mean(X, axis=1)
ys = np.std(X, axis=1)

fig = plt.figure()
ax = fig.add_subplot(111)
line, = ax.plot(xs, ys, 'o', markerfacecolor="none")

plt.show()

One solution is to use "to_rgba" of colorConverter in "matplotlib\backends\qt_editor\figureoptions.py" rather than "to_rgb". It is because "to_rgb" does not handle the "none" option as a transparent color, whereas "to_rgba" does.

# Before
color = rgb2hex(colorConverter.to_rgb(line.get_color()))
ec = rgb2hex(colorConverter.to_rgb(line.get_markeredgecolor()))
fc = rgb2hex(colorConverter.to_rgb(line.get_markerfacecolor()))

# Fixed
color = rgb2hex(colorConverter.to_rgba(line.get_color()))
ec = rgb2hex(colorConverter.to_rgba(line.get_markeredgecolor()))
fc = rgb2hex(colorConverter.to_rgba(line.get_markerfacecolor()))

@tacaswell
Copy link
Member

@dwgoon Thanks for the patch!

attn @anntzer I thought all of the colorConveter uses got removed already. Did the merge to master get messed up or is there another open PR?

@tacaswell tacaswell added this to the 2.0 (style change major release) milestone Jun 15, 2016
@anntzer
Copy link
Contributor

anntzer commented Jun 15, 2016

#6383 (which handles this and a few more updates to the UI code) has not been merged yet. It should be kept under the same milestone as #6382 (new color conversion API).

@tacaswell
Copy link
Member

The milestones got a bit crossed as we backported #6382 to 2.0.

@dwgoon
Copy link
Author

dwgoon commented Jun 15, 2016

Thanks for the reviews :)

@anntzer
Copy link
Contributor

anntzer commented Jun 15, 2016

I guess it's a good test as to whether I actually broke backwards compatibility :) but it probably makes sense to just merge-and-backport #6383 too.

@dwgoon
Copy link
Author

dwgoon commented Jun 15, 2016

It already has a solution! I wish it will be applied as soon as possible.

@QuLogic QuLogic mentioned this pull request Jul 10, 2016
@tacaswell
Copy link
Member

Closing as covered by #6383 and friends.

@tacaswell tacaswell closed this Jul 10, 2016
@QuLogic QuLogic modified the milestones: unassigned, 2.0 (style change major release) Jul 10, 2016
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.

5 participants