Skip to content

line color='none' regression in 1.3 #2760

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
jakevdp opened this issue Jan 24, 2014 · 7 comments
Closed

line color='none' regression in 1.3 #2760

jakevdp opened this issue Jan 24, 2014 · 7 comments

Comments

@jakevdp
Copy link
Contributor

jakevdp commented Jan 24, 2014

Hi,
The following code works in version 1.2, but leads to an exception in version 1.3:

>>> from matplotlib import pyplot as plt
>>> plt.plot(range(10), color='none')
>>> plt.show()

I think the issue boils down to this commit: edc48f0

I'm not sure whether this was an intentional change. Thanks!

@tacaswell
Copy link
Member

Looking at this very quickly (and trying to remember what I did a year ago), it looks like I copied the code a few lines above _get_rgba_face which turns 'none' -> None.

I think the fix is to add isRGBA=True to the set_foreground calls, give me a bit to test.

tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jan 24, 2014
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jan 24, 2014
Fixes matplotlib#2760

Fixes miss-handling of color conversion logic introduced in edc48f0
@efiring
Copy link
Member

efiring commented Jan 24, 2014

Just a caution, without inspecting the code in question: the general rule in mpl is that None means use the default, and "none" means no color, so when I see the latter being converted to the former, it looks suspect.

@mdboom
Copy link
Member

mdboom commented Jan 24, 2014

I had the same thought as @efiring here. I think the path of least change here is (which restores the use of 'none'), but as @tacaswell has looked at this more deeply and more recently than I, I hesitate to turn it into a PR until we've looked at possibly better alternatives:

diff --git a/lib/matplotlib/lines.py b/lib/matplotlib/lines.py
index 42df262..751b24b 100644
--- a/lib/matplotlib/lines.py
+++ b/lib/matplotlib/lines.py
@@ -536,7 +536,8 @@ class Line2D(Artist):

         ln_color_rgba = self._get_rgba_ln_color()
         gc.set_foreground(ln_color_rgba)
-        gc.set_alpha(ln_color_rgba[3])
+        if ln_color_rgba != 'none':
+            gc.set_alpha(ln_color_rgba[3])

         gc.set_antialiased(self._antialiased)
         gc.set_linewidth(self._linewidth)
@@ -1004,7 +1005,7 @@ class Line2D(Artist):
     def _get_rgb_face(self, alt=False):
         facecolor = self._get_markerfacecolor(alt=alt)
         if is_string_like(facecolor) and facecolor.lower() == 'none':
-            rgbFace = None
+            rgbFace = 'none'
         else:
             rgbFace = colorConverter.to_rgb(facecolor)
         return rgbFace
@@ -1012,7 +1013,7 @@ class Line2D(Artist):
     def _get_rgba_face(self, alt=False):
         facecolor = self._get_markerfacecolor(alt=alt)
         if is_string_like(facecolor) and facecolor.lower() == 'none':
-            rgbaFace = None
+            rgbaFace = 'none'
         else:
             rgbaFace = colorConverter.to_rgba(facecolor, self._alpha)
         return rgbaFace
@@ -1020,7 +1021,7 @@ class Line2D(Artist):
     def _get_rgba_ln_color(self, alt=False):
         ln_color = self._color
         if is_string_like(ln_color) and ln_color.lower() == 'none':
-            rgba_ln = None
+            rgba_ln = 'none'
         else:
             rgba_ln = colorConverter.to_rgba(ln_color, self._alpha)
         return rgba_ln

@WeatherGod
Copy link
Member

Your suggestion would make rgba_ln indexable, and would silently work in subsequent code that indexes what it thinks is a 3 or 4 element tuple.

@tacaswell
Copy link
Member

I started down that path last night as well, but it generates a bunch of duplicate code (both in LoC and at run time) as the logic for turning 'none' into 'no-color' is done in colorConverter.to_rgba. If the isRGBA flag is False, then set_foreground calls the to_rgba on whatever is passed in, which in all the cases but 'none' is redundant. By having _get_rgba_ln_color always return an RGBA and the use of the flag, this can be short-circuited.

I am wary of changing the behavior in _get_rgba_face as they were like that when I added _get_rgba_ln_color (and committed some cargo-cult style programming). My guess is that some of face-coloring logic uses None to mean 'no color' someplace down-stream.

@tacaswell
Copy link
Member

closing this as #2761 has been merged. @jakevdp Can you confirm that this fixes your problem?

@jakevdp
Copy link
Contributor Author

jakevdp commented Jan 29, 2014

I'm trying to confirm, but when I import matplotlib after re-installing it, I get

Library not loaded: libfreetype.6.dylib
  Referenced from: /Users/jakevdp/anaconda/envs/mpltest/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-macosx-10.5-x86_64.egg/matplotlib/ft2font.so
  Reason: image not found

It's probably my fault, but I don't have time to dig into this right now. If the test script above works for you, then it probably solves my problem.

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

No branches or pull requests

5 participants