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

ENH: plot only numeric data and raise an exception *before* plotting if there is no numeric data #3572

Merged
merged 2 commits into from May 21, 2013

Conversation

@cpcloud
Copy link
Member

commented May 11, 2013

Raise a TypeError alerting the user to the fact that they are trying to plot nonnumeric data, or if there are any numeric data plot those. closes #1818.

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 14, 2013

I think this should raise before plotting anything... @jreback @y-p thoughts? It's annoying for a plot to come up only to discover that it won't work anyway.

@jreback

This comment has been minimized.

Copy link
Contributor

commented May 14, 2013

@cpcloud I think there is a flag raise_on_error which does this, @changhiskhan put this is a while back...?

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 14, 2013

raise_on_error only gets called after a plot comes up.

@jreback

This comment has been minimized.

Copy link
Contributor

commented May 14, 2013

oh
thought that was the point of it
ok then

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 14, 2013

raise_on_error is a way to catch an exception and show the message, it was only being called for nonnumeric data, so i essentially just replaced it with a typeerror which i thought was clearer.

@jreback

This comment has been minimized.

Copy link
Contributor

commented May 14, 2013

ok...so user can still set raise_on_error to False if they know what they are doing I guess

pls add release notes!

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 16, 2013

@jreback @y-p any thoughts here? i could do something like

if isinteractive():
    ioff()
    the_one_true_plotting_function() # catch the error here before drawing
    ion()
draw_if_interactive()
@jreback

This comment has been minimized.

Copy link
Contributor

commented May 16, 2013

I dont' use this enough to give you an opinion here, but looks reasonable

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 16, 2013

nvm. i can use the currently-unused-method _compute_plot_data to check...

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 16, 2013

i think plotting only numeric data would be a better solution to this problem. have to check if any issues w/ groupby.plot(). will leave open until 0.12, because Series needs to be an NDFrame (it needs the blkmgr method _get_numeric_data)

@jreback

This comment has been minimized.

Copy link
Contributor

commented May 20, 2013

this ready to merge?

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 21, 2013

not just yet

@jreback

This comment has been minimized.

Copy link
Contributor

commented May 21, 2013

FYI this could be done by checking the series dtype but prob better off in. 0.12

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 21, 2013

:) that's what i did, but since it's not critical it might be better to wait

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 21, 2013

also that doesn't cover the case of an object array of numbers which means you have to check the whole array for numbers which is linear in the length of the array which is probably not desirable if u want to plot something gigantic although maybe not the worst thing.

@jreback

This comment has been minimized.

Copy link
Contributor

commented May 21, 2013

actually since u already know the dtype that shouldn't be a problem except in the case of object
where you can just astype in a try except

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 21, 2013

ah good call didn't think of that

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 21, 2013

should i do that and submit? that should work in 0.12 too right? there will still be astype i presume

@jreback

This comment has been minimized.

Copy link
Contributor

commented May 21, 2013

sure

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 21, 2013

guess that allows Series(['1.1', '1.2']).plot() to work as well. that ok?

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 21, 2013

i could the 'safe' argument to disallow that

@jreback

This comment has been minimized.

Copy link
Contributor

commented May 21, 2013

maybe should put a warning on trying to plot an object type in general (if it succeeds that is); otherwise it will raise
in general object type is bad

@jreback

This comment has been minimized.

Copy link
Contributor

commented May 21, 2013

I think object type should warn or raise

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 21, 2013

probably raise to make the user explicitly choose to cast it if they want

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 21, 2013

also raise on error should probably be taken out since that's more of a stop gap than this, although that is an api change

@jreback

This comment has been minimized.

Copy link
Contributor

commented May 21, 2013

yep raise on object type is prob easiest thing to do

@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 21, 2013

fyi...after this merge the raise_on_error argument will be unreachable

@jreback

This comment has been minimized.

Copy link
Contributor

commented May 21, 2013

then go ahead and take it out and add a note in API

cpcloud added 2 commits May 11, 2013
ENH: plot only numeric data
does not raise anymore if there exists some valid data

forgot a test
ENH/API: remove raise_on_error in plotting functions
trivial doc fix

note the docs

revert back
@cpcloud

This comment has been minimized.

Copy link
Member Author

commented May 21, 2013

@jreback this is ready 2 go after teh travis buildz

jreback added a commit that referenced this pull request May 21, 2013
Merge pull request #3572 from cpcloud/plot-nonnumeric-data-exc-1818
ENH: plot only numeric data and raise an exception *before* plotting if there is no numeric data

@jreback jreback merged commit 03d2d30 into pandas-dev:master May 21, 2013

@cpcloud cpcloud deleted the cpcloud:plot-nonnumeric-data-exc-1818 branch May 22, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.