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

BUG: fix handling of color argument for variety of plotting functions #6956

Merged
merged 2 commits into from
May 5, 2014
Merged

Conversation

josham
Copy link
Contributor

@josham josham commented Apr 24, 2014

parallel_coordinates

  • fix reordering of class column (from set) causing possible color/class
    mismatch
  • deprecated use of argument colors in favor of color

radviz

  • fix reordering of class column (from set) causing possible color/class
    mismatch
  • added explicit color keyword argument (avoids multiple values 'color' being
    passed to plotting method)

andrews_curves

  • added explicit color keyword argument (avoids multiple values 'color' being
    passed to plotting method)

To recreate the bug:

import pandas as pd
from pandas.tools.plotting import parallel_coordinates, radviz, andrews_curves

x = pd.DataFrame([[1,2,3,'b'],
                  [2,2,1,'g'],
                  [3,3,1,'r']], columns=[0,1,2,'name'])

parallel_coordinates(x, 'name', colors=['b','g','r'])

# TypeError from scatter
radviz(x, 'name', color=['b','g','r'])

# TypeError from plot
andrews_curves(x, 'name', color=['b','g','r'])

parallel_coordinates before:
ppl_old

parallel_coordinates after:
ppl_new

@@ -438,20 +439,23 @@ def normalize(series):
return ax


def andrews_curves(data, class_column, ax=None, samples=200, colormap=None,
**kwds):
def andrews_curves(frame, class_column, ax=None, samples=200, color=None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check if the deprecate_kwarg decorator works for the change from data to frame? It may not since its a positional argument. If it doesn't you should change frame back to data since this would break code using andrews_curves(data=df, class_column='Name').

@TomAugspurger
Copy link
Contributor

Thanks for the PR. I had those couple comments inline. Could you also add the note you have in doc/source/release.rst to v0.14 under the API section. And you can add a note to the bug section about the other bugs you fixed.

Does this close a particular Github issue?

I'll add this deprecation to #6581 when we merge.

@TomAugspurger
Copy link
Contributor

Also it doesn't look like Travis ran. You may need to hook it up to your Github account: http://about.travis-ci.org/docs/user/getting-started/ You should just have to sign in.

@josham
Copy link
Contributor Author

josham commented May 1, 2014

I made the changes you suggested.

A Travis build seemed to get launched once I pushed my commit but there we errors that seem unrelated to my changes. Do you have any suggestions on what might be the issue? Here is the build https://travis-ci.org/anomrake/pandas/builds/24206970

@jreback
Copy link
Contributor

jreback commented May 1, 2014

pls rebase to master. travis made some changes over the last couple of days, all fixed now.

travis will automatically build on each commit.

@jreback jreback added this to the 0.14.0 milestone May 1, 2014
@josham
Copy link
Contributor Author

josham commented May 1, 2014

I tried to rebase but I'm new to this and I'm a bit surprised to see these other commits show up. Any help would be great.

@jreback
Copy link
Contributor

jreback commented May 1, 2014

see here: https://github.com/pydata/pandas/wiki/Using-Git

git rebase -i origin/master

then

git push yourfork yourbranch -f

@josham
Copy link
Contributor Author

josham commented May 2, 2014

Looks to have worked now. Thanks for the help.

Also, as far as I know, there was no Github issue for the bugs.

@@ -464,6 +464,14 @@ Deprecations
returned if possible, otherwise a copy will be made. Previously the user could think that ``copy=False`` would
ALWAYS return a view. (:issue:`6894`)

- The :func:`parallel_coordinates` function now takes argument ``color``
instead of ``colors``. A ``FutureWarning`` is raised to alert that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls move these to the Plotting sub-section

@josham
Copy link
Contributor Author

josham commented May 3, 2014

I made the change you requested

if ax is None:
ax = plt.gca(xlim=(-pi, pi))
for i in range(n):
row = [columns[c][i] for c in range(len(columns))]
row = df.irow(i).values
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed this the first time. I think we're deprecating irow and icol sometime. Could you change this to df.iloc[i].values

@TomAugspurger
Copy link
Contributor

If you could rebase one last time and change those irows to icols, I'll get this merged. Thanks.

parallel_coordinates
- fix reordering of class column (from set) causing possible color/class
  mismatch
- deprecated use of argument colors in favor of color
radviz
- fix reordering of class column (from set) causing possible color/class
  mismatch
- added explicit color keyword argument (avoids multiple values 'color' being
  passed to plotting method)
andrews_curves
- added explicit color keyword argument (avoids multiple values 'color' being
  passed to plotting method)
parallel_coordinates/andrews_curves
- added deprecate_kwarg decorator for using frame argument instead of data
- added tests to check that FutureWarning is raised properly
@josham
Copy link
Contributor Author

josham commented May 5, 2014

OK done

TomAugspurger pushed a commit that referenced this pull request May 5, 2014
BUG: fix handling of color argument for variety of plotting functions
@TomAugspurger TomAugspurger merged commit de6ed81 into pandas-dev:master May 5, 2014
@TomAugspurger
Copy link
Contributor

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants