Skip to content

BUG: Fix #25481 by fixing the error message in TypeError #25540

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Mar 10, 2019
Merged

BUG: Fix #25481 by fixing the error message in TypeError #25540

merged 5 commits into from
Mar 10, 2019

Conversation

bharatr21
Copy link
Contributor

@bharatr21 bharatr21 commented Mar 5, 2019 •
edited
Loading

BUG: Fix #25481 by fixing the error message in TypeError

@simonjayhawkins
Copy link
Member

@Bharat123rox : a bug fix requires a whatsnew entry... and maybe more tests.

these tests hit the line of code changed..

pandas/tests/plotting/test_datetimelike.py::TestTSPlot::test_nonnumeric_exclude 
pandas/tests/plotting/test_frame.py::TestDataFramePlots::test_subplots_timeseries_y_axis 
pandas/tests/plotting/test_frame.py::TestDataFramePlots::test_all_invalid_plot_data
pandas/tests/plotting/test_frame.py::TestDataFramePlots::test_partially_invalid_plot_data
pandas/tests/plotting/test_series.py::TestSeriesPlots::test_invalid_plot_data
pandas/tests/plotting/test_series.py::TestSeriesPlots::test_partially_invalid_plot_data

so it may be possible to just use the match parameter of pytest.raises in these tests.

@codecov
Copy link

codecov bot commented Mar 5, 2019

Codecov Report

Merging #25540 into master will decrease coverage by 50.04%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #25540       +/-   ##
===========================================
- Coverage   91.75%   41.71%   -50.05%     
===========================================
  Files         173      173               
  Lines       52960    52960               
===========================================
- Hits        48596    22091    -26505     
- Misses       4364    30869    +26505
Flag Coverage Δ
#multiple ?
#single 41.71% <0%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/plotting/_core.py 17.79% <0%> (-65.8%) ⬇️
pandas/io/formats/latex.py 0% <0%> (-100%) ⬇️
pandas/core/categorical.py 0% <0%> (-100%) ⬇️
pandas/io/sas/sas_constants.py 0% <0%> (-100%) ⬇️
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
pandas/io/formats/html.py 0% <0%> (-99.36%) ⬇️
pandas/core/groupby/categorical.py 0% <0%> (-95.46%) ⬇️
pandas/io/sas/sas7bdat.py 0% <0%> (-91.17%) ⬇️
pandas/io/sas/sas_xport.py 0% <0%> (-90.15%) ⬇️
... and 131 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 076b5a8...f48d2f6. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 5, 2019 •
edited
Loading

Codecov Report

Merging #25540 into master will decrease coverage by 0.49%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #25540     +/-   ##
=========================================
- Coverage   91.75%   91.26%   -0.5%     
=========================================
  Files         173      173             
  Lines       52960    52966      +6     
=========================================
- Hits        48596    48337    -259     
- Misses       4364     4629    +265
Flag Coverage Δ
#multiple 89.83% <100%> (-0.5%) ⬇️
#single 41.71% <0%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/plotting/_core.py 83.58% <100%> (ø) ⬆️
pandas/core/panel.py 38.56% <0%> (-33.19%) ⬇️
pandas/core/sparse/series.py 93.3% <0%> (-2.24%) ⬇️
pandas/core/indexing.py 90.88% <0%> (-1.41%) ⬇️
pandas/core/internals/managers.py 93.92% <0%> (-0.92%) ⬇️
pandas/core/generic.py 93.65% <0%> (-0.51%) ⬇️
pandas/core/ops.py 91.63% <0%> (-0.14%) ⬇️
pandas/util/testing.py 87.57% <0%> (-0.1%) ⬇️
pandas/core/frame.py 96.79% <0%> (-0.06%) ⬇️
pandas/io/formats/style.py 96.69% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 076b5a8...83aaf2c. Read the comment docs.

TomAugspurger
TomAugspurger reviewed Mar 5, 2019
View reviewed changes
doc/source/whatsnew/v0.25.0.rst Outdated
~~~~~~~~~
- Bug in :func:`to_datetime` which would raise an (incorrect) ``ValueError`` when called with a date far into the future and the ``format`` argument specified instead of raising ``OutOfBoundsDatetime`` (:issue:`23830`)
-
- Bug in :func: `plot` for a `pd.Series` or a `pd.DataFrame` which had a confusing error message mentioning an "Empty" DataFrame, now raises ``TypeError: no numeric data to plot`` (:issue:`25481`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just refer to it as :meth:`DataFrame.plot`.

Probably don't need to include the error message here. Rather, say something like "Improved the error message of ..."

@TomAugspurger TomAugspurger added the Error Reporting Incorrect or improved errors from pandas label Mar 5, 2019
@TomAugspurger TomAugspurger added this to the 0.25.0 milestone Mar 5, 2019
@jreback
Copy link
Contributor

jreback commented Mar 5, 2019

lgtm. @TomAugspurger

WillAyd
WillAyd reviewed Mar 6, 2019
View reviewed changes
pandas/plotting/_core.py Outdated
except AttributeError:
is_empty = not len(numeric_data)

# no empty frames or series allowed
Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker for me but might be nice to clarify this comment. On initial glance it implies to me that the entire passed Frame might have been empty, whereas it really only y applies where numeric data doesn't exist, which is slightly different

@jreback jreback merged commit 16edaaf into pandas-dev:master Mar 10, 2019
@jreback
Copy link
Contributor

jreback commented Mar 10, 2019

thanks @Bharat123rox

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WillAyd WillAyd WillAyd left review comments

@TomAugspurger TomAugspurger TomAugspurger left review comments

Assignees
No one assigned
Labels
Error Reporting Incorrect or improved errors from pandas
Projects
None yet
Milestone
0.25.0
Development

Successfully merging this pull request may close these issues.

BUG: confusing error message for DataFrame.plot()
5 participants
@bharatr21 @simonjayhawkins @jreback @WillAyd @TomAugspurger