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

to_rgb(float) or to_rgb(str(flot)) #2609

Closed
Carreau opened this issue Nov 21, 2013 · 3 comments · Fixed by #2610
Closed

to_rgb(float) or to_rgb(str(flot)) #2609

Carreau opened this issue Nov 21, 2013 · 3 comments · Fixed by #2610

Comments

@Carreau
Copy link
Contributor

Carreau commented Nov 21, 2013

According to the doc of matplotlib.colors.ColorConverter.to_rgb

Returns an RGB tuple of three floats from 0-1.

arg can be an RGB or RGBA sequence or a string in any of several forms:
...
a float, like ‘0.4’, indicating gray on a 0-1 scale
...

but

In [12]: matplotlib.colors.colorConverter.to_rgb(0.4)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-12-7cac10830aab> in <module>()
----> 1 matplotlib.colors.colorConverter.to_rgb(0.4)

/Users/bussonniermatthias/matplotlib/lib/matplotlib/colors.pyc in to_rgb(self, arg)
    323         except (KeyError, ValueError, TypeError) as exc:
    324             raise ValueError(
--> 325                 'to_rgb: Invalid rgb arg "%s"\n%s' % (str(arg), exc))
    326             # Error messages could be improved by handling TypeError
    327             # separately; but this should be rare and not too hard

ValueError: to_rgb: Invalid rgb arg "0.4"
cannot convert argument to rgb sequence

In [13]: matplotlib.colors.colorConverter.to_rgb("0.4")
Out[13]: (0.4, 0.4, 0.4)

Should I correct/improve the doc ? or fix to_rgb to accept floats too ?
Or am I missing something ?

pretty recent version:

commit f8687b08df74e702de72621952bfb911719aaf26
Merge: 6b1d8e1 514bac9
Date:   Wed Nov 20 19:22:04 2013 -0800
    Merge pull request #2603 from fariza/aspect_auto_warning_bug
@mdboom
Copy link
Member

mdboom commented Nov 21, 2013

I don't immediately see any harm in making this accept a float. Hopefully the test suite will tell us if it's a problem....

@efiring
Copy link
Member

efiring commented Nov 21, 2013

Careful! I think this is a bad idea. Please just fix the documentation to make it consistent with the colors.py module docstring. Long ago we allowed both a float and a string, and it caused major problems--specifically, it makes it impossible to distinguish between an array of three greys and a single RGB tuple. Therefore we decreed that only a string representation of a float would be allowed to represent a grey value.

@efiring
Copy link
Member

efiring commented Nov 21, 2013

Note that the existing to_rgb docstring does show the float as a string--the only ambiguity is whether that is done to set it apart from the rest of the sentence, or whether it means "a string representation of a float", which was the intention. So, you could explicitly disambiguate this, replacing "a float" with "a string representation of a float".

Carreau added a commit to Carreau/matplotlib that referenced this issue Nov 22, 2013
Even if the docstring was saing it, emphasis that to_rgb/to_rgba do not
accept float as input, but string representation of a float.

As one might be tempted to had float->gray conversion add failing test
in case the functionality is added.

Side fixes :
    change tuple([value]*3) to (value,)*3 for speed

In [18]: %timeit  tuple([0.4]*3)
1000000 loops, best of 3: 505 ns per loop

In [19]: %timeit  (0.4,)*3
10000000 loops, best of 3: 23.5 ns per loop

In [20]:  (0.4,)*3 == tuple([0.4]*3)
Out[20]: True

Closes matplotlibgh-2609
Carreau added a commit to Carreau/matplotlib that referenced this issue Nov 24, 2013
Even if the docstring was saing it, emphasis that to_rgb/to_rgba do not
accept float as input, but string representation of a float.

As one might be tempted to had float->gray conversion add failing test
in case the functionality is added.

Side fixes :
    change tuple([value]*3) to (value,)*3 for speed

In [18]: %timeit  tuple([0.4]*3)
1000000 loops, best of 3: 505 ns per loop

In [19]: %timeit  (0.4,)*3
10000000 loops, best of 3: 23.5 ns per loop

In [20]:  (0.4,)*3 == tuple([0.4]*3)
Out[20]: True

Closes matplotlibgh-2609
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 a pull request may close this issue.

3 participants