BUG: allow numpy.array as c values to scatterplot #8929

aevri · 2014-11-29T16:32:11Z

Ensure that we can pass an np.array as 'c' straight through to
matplotlib, this functionality was accidentally removed previously.

Add tests.

Closes #8852

shoyer · 2014-11-29T16:51:09Z

pandas/core/common.py

@@ -2500,6 +2500,31 @@ def is_list_like(arg):
            not isinstance(arg, compat.string_and_binary_types))


+def is_hashy(arg):


looks pretty good to me, though I would call this is_hashable rather than is_hashy

yep is_hashable is better

I was thinking that originally, I felt uncomfortable because of the surprising implementation.

Would is_safely_hashable be good maybe?

Thanks!

Personally, I would not expect is_hashable to use collections.Hashable (if you wanted that, you could just do the isinstance check). So this implementation does not surprise me, at least. Also, I'm pretty sure the Python docs explicitly state that __hash__ is supposed to be cheap to evaluate.

Ok cool, have rebased and renamed to is_hashable, thanks for explaining

jreback · 2014-11-29T21:33:23Z

doc/source/whatsnew/v0.15.2.txt

@@ -145,6 +145,8 @@ Bug Fixes
 - BUG: Option context applies on __enter__ (:issue:`8514`)


+- Bug where plot with kind=scatter fails when checking if an array is in the DataFrame (:issue:`8852`)


say DataFrame.plot(kind='scatter')

jreback · 2014-11-29T21:34:32Z

pls squash, otherwise looks ok

@TomAugspurger ?

TomAugspurger · 2014-11-29T21:54:52Z

pandas/tests/test_graphics.py

+        #
+        df = DataFrame({'A': [1, 2], 'B': [3, 4]})
+        c = np.array([1, 0])
+        df.plot(kind='scatter', x='A', y='B', c=c)


Can you check that the colors are correct? Get back the result from plot with ax = df.plot(kind='scatter', x='A', y='B', c=c) and get the collections attribute, and check ax.collections[0].get_facecolor() against the expected array([[ 1., 1., 1., 1.], [ 0., 0., 0., 1.]])

@TomAugspurger Is that the expected color? I think the [1, 0] are interpreted as numeric values that are scaled to the default colormap, not as rgb colors? (when you do c=[[1,1,1],[0,0,0]] matplotlib does something different)

That's what I get for fig, ax = plt.subplots(); ax.scatter([0, 0], [1, 1], c=[0, 1])

c can be a single color format string, or a sequence of color specifications of length N, or a sequence of N numbers to be mapped to colors using the cmap and norm specified via kwargs (see below). Note that c should not be a single numeric RGB or RGBA sequence because that is indistinguishable from an array of values to be colormapped. c can be a 2-D array in which the rows are RGB or RGBA, however.

from http://matplotlib.org/api/pyplot_api.html#matplotlib.pyplot.scatter

We should handle the same if possible.

Weird that the order is flipped if you do

ax.scatter([0, 0], [1, 1], c=[0, 1]) vs ax.scatter([0, 0], [1, 1], c=[[0, 0, 0, 1], [1, 1, 1, 1]])

Hmm, because I did the same and got colors and not black/white. You don't have a custom matplotlib defaults that sets cmap at grayscale? (maybe seaborn does that?)

I have seaborn imported in my IPython profile.

TomAugspurger · 2014-11-29T21:56:48Z

I'm fine with this being two commits since it separates the addition of is_hashable and the plotting issue.

Could you add a test or two for is_hashable too? Thanks.

jreback · 2014-11-29T21:57:41Z

@aevri pls add test for is_hashable in tests/test_common

aevri · 2014-11-30T17:26:30Z

I've updated with these changes:

added tests for is_hashable in tests/test_common
modified is_hashable to first reject based on collections.Hashable, this is to avoid broadening the definition of hashable to include old-style classes and anything else I can't think of
changed whatsnew entry to say DataFrame.plot(kind='scatter')
In scatter_with_c tests, supplied RGBA colors as 'c' and added test for face colors to be identical to supplied values
rebased to catch up with upstream

Was considering how to test the face colors of the case where we supply a colormap, don't know how to query the colormap ahead of time though.

Cheers!

jorisvandenbossche · 2014-11-30T23:26:10Z

@aevri By the way, David (@cournape) fixed it in numpy today: numpy/numpy#5326

And the tests are failing on python 3 (and on numpy master, because of Davids fix)

aevri · 2014-12-01T07:41:15Z

Have updated:

rebased on master to fix merge conflicts
wrapped Python2-specific tests in sys.version_info check
made numpy.array test work with or without numpy fix

Cheers!

Some types will pass a test against collections.Hashable but fail when they are actually hashed with hash(). Introduce a new predicate to help handle these types. Will use this in subsequent commit. Test Plan: In addition to running tests, run the doctest $ nosetests pandas/core/common.py:is_hashable --with-doc -v

Ensure that we can pass an np.array as 'c' straight through to matplotlib, this functionality was accidentally removed previously. Add tests. Closes pandas-dev#8852

aevri · 2014-12-02T22:59:41Z

Rebased again for new merge conflicts

Afaik all points are addressed and this is ready to land, please let me know if there's something I've missed :O)

jorisvandenbossche · 2014-12-02T23:05:29Z

Looks OK for me! Lets wait until travis is green

jreback · 2014-12-03T22:34:20Z

@aevri thanks for the fix!

aevri · 2014-12-03T22:38:46Z

Yay! Thanks everyone :)

shoyer reviewed Nov 29, 2014
View reviewed changes

aevri force-pushed the fix/8852 branch from 5059403 to 49f4e94 Compare November 29, 2014 19:55

jreback added Bug Visualization plotting labels Nov 29, 2014

jreback added this to the 0.15.2 milestone Nov 29, 2014

jreback reviewed Nov 29, 2014
View reviewed changes

TomAugspurger reviewed Nov 29, 2014
View reviewed changes

aevri force-pushed the fix/8852 branch from 49f4e94 to eb6e363 Compare November 30, 2014 17:19

aevri force-pushed the fix/8852 branch 2 times, most recently from 14fac02 to 9881d87 Compare December 1, 2014 00:58

aevri added 2 commits December 2, 2014 22:53

BUG: allow numpy.array as c values to scatterplot

bd68912

Ensure that we can pass an np.array as 'c' straight through to matplotlib, this functionality was accidentally removed previously. Add tests. Closes pandas-dev#8852

aevri force-pushed the fix/8852 branch from 9881d87 to bd68912 Compare December 2, 2014 22:54

jreback merged commit bd68912 into pandas-dev:master Dec 3, 2014

jorisvandenbossche mentioned this pull request Dec 5, 2014

ENH/VIS: Pass DataFrame column to size argument in DataFrame.scatter #8885

Closed

jorisvandenbossche mentioned this pull request Jan 5, 2015

Validate that 'name' attribute is set only if hashable #9193

Closed

shoyer mentioned this pull request Jan 16, 2015

BUG: common.is_hashable returns False for np.float64 on Python 3 #9276

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: allow numpy.array as c values to scatterplot #8929

BUG: allow numpy.array as c values to scatterplot #8929

aevri commented Nov 29, 2014

shoyer Nov 29, 2014

jreback Nov 29, 2014

aevri Nov 29, 2014

shoyer Nov 29, 2014

aevri Nov 29, 2014

jreback Nov 29, 2014

jreback commented Nov 29, 2014

TomAugspurger Nov 29, 2014

jorisvandenbossche Nov 29, 2014

TomAugspurger Nov 30, 2014

TomAugspurger Nov 30, 2014

TomAugspurger Nov 30, 2014

jorisvandenbossche Nov 30, 2014

TomAugspurger Nov 30, 2014

TomAugspurger commented Nov 29, 2014

jreback commented Nov 29, 2014

aevri commented Nov 30, 2014

jorisvandenbossche commented Nov 30, 2014

aevri commented Dec 1, 2014

aevri commented Dec 2, 2014

jorisvandenbossche commented Dec 2, 2014

jreback commented Dec 3, 2014

aevri commented Dec 3, 2014

		@@ -2500,6 +2500,31 @@ def is_list_like(arg):
		not isinstance(arg, compat.string_and_binary_types))


		def is_hashy(arg):

		@@ -145,6 +145,8 @@ Bug Fixes
		- BUG: Option context applies on __enter__ (:issue:`8514`)


		- Bug where plot with kind=scatter fails when checking if an array is in the DataFrame (:issue:`8852`)

BUG: allow numpy.array as c values to scatterplot #8929

BUG: allow numpy.array as c values to scatterplot #8929

Conversation

aevri commented Nov 29, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Nov 29, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomAugspurger commented Nov 29, 2014

jreback commented Nov 29, 2014

aevri commented Nov 30, 2014

jorisvandenbossche commented Nov 30, 2014

aevri commented Dec 1, 2014

aevri commented Dec 2, 2014

jorisvandenbossche commented Dec 2, 2014

jreback commented Dec 3, 2014

aevri commented Dec 3, 2014