Skip to content

improved error message for invalid chart types #9417

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

Merged
merged 1 commit into from
Feb 8, 2015

Conversation

cel4
Copy link
Contributor

@cel4 cel4 commented Feb 4, 2015

Closes #9400

pd.Series([2,3,4]).plot("quiver") now gives an improved error message:

Users/ch/miniconda/envs/pd_dev34/lib/python3.4/site-packages/pandas/tools/plotting.py in _plot(data, x, y, subplots, ax, kind, **kwds)
   2269         klass = _plot_klass[kind]
   2270     else:
-> 2271         raise ValueError(invalid_ct_fmtstr % kind)
   2272 
   2273     from pandas import DataFrame

ValueError: 'quiver' is not a valid chart type

instead of:

/Users/ch/miniconda/envs/sci33/lib/python3.3/site-packages/pandas/tools/plotting.py in _plot(data, x, y, subplots, ax, kind, **kwds)
   2267         klass = _plot_klass[kind]
   2268     else:
-> 2269         raise ValueError('Invalid chart type given %s' % kind)
   2270 
   2271     from pandas import DataFrame

ValueError: Invalid chart type given quiver

@@ -2262,11 +2262,13 @@ def result(self):

def _plot(data, x=None, y=None, subplots=False,
ax=None, kind='line', **kwds):

invalid_ct_fmtstr = "'%s' is not a valid chart type"
Copy link
Member

Choose a reason for hiding this comment

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

I would use the word kind instead of type to match the keyword argument.

Also, perhaps use %r instead of '%s' to properly format non-string arguments, too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would you formulate the message? 'blabla' is not a valid value for the keyword argument 'kind'?

Is it possible to replace chart type by chart kind? I am not a native speaker - to me it somehow sounds strange.

at this point kind can only be a string. It gets set in line 2265: kind = _get_standard_kind(kind.lower().strip())

Copy link
Member

Choose a reason for hiding this comment

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

Maybe "%r is not a valid plot kind"? I see you are correct about the type.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm... I don't really have a strong opinion here about the exact wording.

In any case this is a clear improvement over the original. If you're happy with the wording that's fine.

@jorisvandenbossche jorisvandenbossche added the Error Reporting Incorrect or improved errors from pandas label Feb 5, 2015
@jorisvandenbossche jorisvandenbossche added this to the 0.16.0 milestone Feb 5, 2015
@@ -2274,7 +2276,7 @@ def _plot(data, x=None, y=None, subplots=False,
plot_obj = klass(data, x=x, y=y, subplots=subplots, ax=ax,
kind=kind, **kwds)
else:
raise ValueError('Invalid chart type given %s' % kind)
raise ValueError(invalid_ct_fmtstr % kind)
Copy link
Member

Choose a reason for hiding this comment

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

I think you should ideally differentiate between the two messages. As the first message is just general 'not a valid plot kind'. While this message is specific for DataFrames (you get here if you give a kind that is only possible for a dataframe, but if your object is not a dataframe). So here I would say something like '%r is only a valid plot kind for DataFrames' or the like.

@jreback jreback added the Visualization plotting label Feb 5, 2015
@cel4
Copy link
Contributor Author

cel4 commented Feb 7, 2015

Can you check the new error messages? If they're ok, I would squash my commits.

@shoyer
Copy link
Member

shoyer commented Feb 8, 2015

This looks good to me. @jorisvandenbossche ?

kind = _get_standard_kind(kind.lower().strip())
if kind in _all_kinds:
klass = _plot_klass[kind]
else:
raise ValueError('Invalid chart type given %s' % kind)
raise ValueError("%r is not a valid plot kind" % kind)
Copy link
Member

Choose a reason for hiding this comment

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

a very small thing, but can you add ' around the %r? I think that makes the message more clear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

%r seems to add the 'automatically: print("%r" % "hi") prints: 'hi'

Copy link
Member

Choose a reason for hiding this comment

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

ah, I didn't know that. Perfect then!

@jorisvandenbossche
Copy link
Member

@cel4 Just a small question, can you squash the two commits into one?

@jorisvandenbossche
Copy link
Member

@cel4 Looking good, thanks! I'll merge when travis is green

jorisvandenbossche added a commit that referenced this pull request Feb 8, 2015
improved error message for invalid chart types
@jorisvandenbossche jorisvandenbossche merged commit 107cb10 into pandas-dev:master Feb 8, 2015
@cel4 cel4 deleted the plot_error branch August 15, 2015 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Error Reporting Incorrect or improved errors from pandas Visualization plotting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve error message in plotting.py's _plot
4 participants