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.

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

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

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
@@ -121,7 +121,7 @@ Performance Improvements
Bug Fixes
~~~~~~~~~
- 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
@@ -360,8 +360,7 @@ def _compute_plot_data(self):

# 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

thoo added a commit to thoo/pandas that referenced this pull request Mar 11, 2019
@thoo
Merge remote-tracking branch 'upstream/master' into pivot
aaa21bb 
* upstream/master: (110 commits)
  DOC: hardcode contributors for 0.24.x releases (pandas-dev#25662)
  DOC: restore toctree maxdepth (pandas-dev#25134)
  BUG: Redefine IndexOpsMixin.size, fix pandas-dev#25580. (pandas-dev#25584)
  BUG: to_csv line endings with compression (pandas-dev#25625)
  DOC: file obj for to_csv must be newline='' (pandas-dev#25624)
  Suppress incorrect warning in nargsort for timezone-aware DatetimeIndex (pandas-dev#25629)
  TST: fix incorrect sparse test (now failing on scipy master) (pandas-dev#25653)
  CLN: Removed debugging code (pandas-dev#25647)
  DOC: require Return section only if return is not None nor commentary (pandas-dev#25008)
  DOC:Remove hard-coded examples from _flex_doc_SERIES (pandas-dev#24589) (pandas-dev#25524)
  TST: xref pandas-dev#25630 (pandas-dev#25643)
  BUG: Fix pandas-dev#25481 by fixing the error message in TypeError (pandas-dev#25540)
  Fixturize tests/frame/test_mutate_columns.py (pandas-dev#25642)
  Fixturize tests/frame/test_join.py (pandas-dev#25639)
  Fixturize tests/frame/test_combine_concat.py (pandas-dev#25634)
  Fixturize tests/frame/test_asof.py (pandas-dev#25628)
  BUG: Fix user-facing AssertionError with to_html (pandas-dev#25608) (pandas-dev#25620)
  DOC: resolve all GL03 docstring validation errors (pandas-dev#25525)
  TST: failing wheel building on PY2 and old numpy (pandas-dev#25631)
  DOC: Remove makePanel from docs (pandas-dev#25609) (pandas-dev#25612)
  ...
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