BUG: fixes formatted value error for missing sheet (#27676) #27677

JasperVanDenBosch · 2019-07-31T15:19:32Z

Fixes a formatted message at io.excel._odfreader line 62

closes read_excel: sheet_name of non-existent sheet does not raise appropriate error for engine='odf' #27676
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

TomAugspurger · 2019-07-31T20:09:30Z

Thanks. Could you release note under "Bug fixes" in doc/source/v0.25.1.rst and a test ensuring that the sheet name is including the the error message?

TomAugspurger · 2019-07-31T20:09:41Z

You can ignore the codecov failure.

WillAyd · 2019-08-01T15:04:38Z

Looks good. Can you check test coverage for this though? Should have one for all excel / odf engines

pandas/tests/io/excel/test_odf.py

WillAyd · 2019-08-01T16:50:05Z

Interesting that all of the engines raise differently... @ilogue do you have an idea of effort to align all of these messages? Would ideally like to do here if not too much effort but could also leave to a follow up if non-trivial

JasperVanDenBosch · 2019-08-01T16:54:46Z

@WillAyd by align do you mean catch the exceptions thrown by the other packages and rethrow a common pandas exception? Or ask the maintainers of those packages to change their error message?

WillAyd · 2019-08-01T16:57:51Z

Yea I mean raise one common error message from get_sheet_by_name in the various excel readers. So for example since xlrd raises an XLRDError would catch and re-raise a ValueError that matches what would happen with openpyxl and odfpy

jreback · 2019-08-04T22:06:41Z

doc/source/whatsnew/v0.25.1.rst

@@ -25,6 +25,7 @@ Other enhancements
 Bug fixes
 ~~~~~~~~~

+- Bug in :meth:`DataFrame.read_excel` with `engine='ods'` when `sheet_name` argument references a non-existent sheet (:issue:`27676`)


can you move this to the IO section, double backticks rather than single around engine='ods' and sheet_name

…der-sheetname-missing

TomAugspurger · 2019-08-21T15:22:54Z

@ilogue looks like the CI is failing. Do these tests pass for you locally?

JasperVanDenBosch · 2019-08-21T16:03:12Z

No they don't work, because they currently target all engines. So we could

Moved the test back to the engine specific module
Implement BUG: fixes formatted value error for missing sheet (#27676) #27677 (comment)

JasperVanDenBosch · 2019-08-21T16:04:03Z

Sorry didn't mean to close, on my phone

TomAugspurger · 2019-08-21T16:40:06Z

Do you have an opinion on which option is better?

pep8speaks · 2019-08-23T12:44:26Z

Hello @ilogue! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-10-25 16:02:56 UTC

WillAyd · 2019-09-19T16:12:00Z

pandas/tests/io/excel/test_odf.py

@@ -36,3 +36,11 @@ def test_read_writer_table():
    result = pd.read_excel("writertable.odt", "Table1", index_col=0)

    tm.assert_frame_equal(result, expected)
+
+
+def test_nonexistent_sheetname_raises(self, read_ext):


The use of self here is causing the error. This isn't a class method so that's what is failing CI.

If you remove I think should turn green

should be done in c45ffb3

This reverts commit 9306864. Duplicates pandas-dev#27677

WillAyd

On phone so can’t check but looks like there is an outstanding CI failure that needs to be addressed. Otherwise lgtm

WillAyd · 2019-10-08T17:54:09Z

doc/source/whatsnew/v0.25.2.rst

@@ -64,7 +64,7 @@ I/O

 - Fix regression in notebook display where <th> tags not used for :attr:`DataFrame.index` (:issue:`28204`).
 - Regression in :meth:`~DataFrame.to_csv` where writing a :class:`Series` or :class:`DataFrame` indexed by an :class:`IntervalIndex` would incorrectly raise a ``TypeError`` (:issue:`28210`)
-
+- Bug in :meth:`DataFrame.read_excel` with ``engine='ods'`` when ``sheet_name`` argument references a non-existent sheet (:issue:`27676`)


Can you move this to 1.0.0?

WillAyd · 2019-10-24T16:52:45Z

@ilogue can you fix up merge conflicts and revert any changes to 0.25.2 whatsnew?

JasperVanDenBosch · 2019-10-24T17:14:01Z

@ilogue can you fix up merge conflicts and revert any changes to 0.25.2 whatsnew?

@WillAyd done

jreback

doc comment, ping on green.

jreback · 2019-10-25T12:48:07Z

doc/source/whatsnew/v1.0.0.rst

@@ -398,6 +399,12 @@ Sparse
 -
 -

+


not sure what this is, pls remove

see https://github.com/pandas-dev/pandas/pull/27677/files/5e92329599adfbd42ee2df0c046eb0f659fcd226#r339034166

WillAyd · 2019-10-25T17:01:36Z

Great thanks @ilogue

…pandas-dev#27677)

BUG: fixes formatted value error for missing sheet

c25f93b

TomAugspurger added this to the 0.25.1 milestone Jul 31, 2019

TomAugspurger added Error Reporting Incorrect or improved errors from pandas IO Excel read_excel, to_excel labels Jul 31, 2019

added reference to bug fix for pandas-dev#27676 in whats new

6665fdb

TST: Add test for read_excel with odf engine and nonexistent sheetname

c83c7c9

WillAyd requested changes Aug 1, 2019

View reviewed changes

pandas/tests/io/excel/test_odf.py Outdated Show resolved Hide resolved

JasperVanDenBosch added 3 commits August 1, 2019 17:22

CLN: black fix in test_odf

406f1a0

TST: sheet_name test now in test_readers

90897c5

TST: test_nonexistent_sheetname_raises required read_ext param

a4efca0

jreback requested changes Aug 4, 2019

View reviewed changes

Merge remote-tracking branch 'upstream/master' into ilogue-fix-odfrea…

4d54e3a

…der-sheetname-missing

TomAugspurger approved these changes Aug 20, 2019

View reviewed changes

review comments

e93b25a

JasperVanDenBosch closed this Aug 21, 2019

JasperVanDenBosch reopened this Aug 21, 2019

TomAugspurger modified the milestones: 0.25.1, 1.0 Aug 21, 2019

moved odf reader error message test back to odf tests

1c2fc81

JasperVanDenBosch added 3 commits September 10, 2019 19:12

Merge branch 'master' into fix-odfreader-sheetname-missing

c149675

reverted changes to v0.25.1 whatsnew

bec56d9

added line for pandas-dev#27676 in whatsnew 0.25.2

a93b355

WillAyd requested changes Sep 19, 2019

View reviewed changes

jreback modified the milestones: 0.25.2, 1.0 Sep 20, 2019

JasperVanDenBosch added 2 commits September 24, 2019 18:03

[TST] fixed odf unittest

c45ffb3

[TST]: odf tests: hardcoded blank workbook extension as .ods

589cbb1

WillAyd mentioned this pull request Oct 8, 2019

[MINOR] Fix formatting string in _ODFReader.get_sheet_by_name #28849

Closed

5 tasks

zero323 added a commit to zero323/pandas that referenced this pull request Oct 8, 2019

Revert "Type ignore _side_expander calls in CSSResolver"

e695827

This reverts commit 9306864. Duplicates pandas-dev#27677

WillAyd requested changes Oct 8, 2019

View reviewed changes

JasperVanDenBosch added 2 commits October 8, 2019 19:06

moved whatsnew entry to 1.0

56cc92d

Merge branch 'master' into fix-odfreader-sheetname-missing

669221d

WillAyd mentioned this pull request Oct 24, 2019

CLN: Add types in a handful of places #29178

Merged

WillAyd mentioned this pull request Oct 24, 2019

Fix missing argument name in format function call #29201

Closed

Merge branch 'master' into fix-odfreader-sheetname-missing

5e92329

jreback requested changes Oct 25, 2019

View reviewed changes

doc/source/whatsnew/v1.0.0.rst Outdated

@@ -398,6 +399,12 @@ Sparse

-

-

Copy link

Contributor

jreback Oct 25, 2019

Choose a reason for hiding this comment

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

not sure what this is, pls remove

JasperVanDenBosch and others added 2 commits October 25, 2019 14:39

Removed whatsnew 1.0 entry for pandas-dev#26838 from merged master

04fb46b

see https://github.com/pandas-dev/pandas/pull/27677/files/5e92329599adfbd42ee2df0c046eb0f659fcd226#r339034166

Update v0.25.2.rst

3d135fb

WillAyd approved these changes Oct 25, 2019

View reviewed changes

WillAyd merged commit d7de334 into pandas-dev:master Oct 25, 2019

JasperVanDenBosch deleted the fix-odfreader-sheetname-missing branch October 28, 2019 13:56

HawkinsBA pushed a commit to HawkinsBA/pandas that referenced this pull request Oct 29, 2019

BUG: fixes formatted value error for missing sheet (pandas-dev#27676) (…

a23a589

…pandas-dev#27677)

Reksbril pushed a commit to Reksbril/pandas that referenced this pull request Nov 18, 2019

BUG: fixes formatted value error for missing sheet (pandas-dev#27676) (…

551c770

…pandas-dev#27677)

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

BUG: fixes formatted value error for missing sheet (pandas-dev#27676) (…

3f5e5e4

…pandas-dev#27677)

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

BUG: fixes formatted value error for missing sheet (pandas-dev#27676) (…

df838f9

…pandas-dev#27677)

bongolegend pushed a commit to bongolegend/pandas that referenced this pull request Jan 1, 2020

BUG: fixes formatted value error for missing sheet (pandas-dev#27676) (…

77c956d

…pandas-dev#27677)

Uh oh!

BUG: fixes formatted value error for missing sheet (#27676) #27677

BUG: fixes formatted value error for missing sheet (#27676) #27677

Uh oh!

Conversation

JasperVanDenBosch commented Jul 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomAugspurger commented Jul 31, 2019

Uh oh!

TomAugspurger commented Jul 31, 2019

Uh oh!

WillAyd commented Aug 1, 2019

Uh oh!

Uh oh!

WillAyd commented Aug 1, 2019

Uh oh!

JasperVanDenBosch commented Aug 1, 2019

Uh oh!

WillAyd commented Aug 1, 2019

Uh oh!

jreback Aug 4, 2019

Choose a reason for hiding this comment

Uh oh!

TomAugspurger commented Aug 21, 2019

Uh oh!

JasperVanDenBosch commented Aug 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JasperVanDenBosch commented Aug 21, 2019

Uh oh!

TomAugspurger commented Aug 21, 2019

Uh oh!

pep8speaks commented Aug 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2019-10-25 16:02:56 UTC

Uh oh!

WillAyd Sep 19, 2019

Choose a reason for hiding this comment

Uh oh!

JasperVanDenBosch Sep 24, 2019

Choose a reason for hiding this comment

Uh oh!

WillAyd left a comment

Choose a reason for hiding this comment

Uh oh!

WillAyd Oct 8, 2019

Choose a reason for hiding this comment

Uh oh!

JasperVanDenBosch Oct 9, 2019

Choose a reason for hiding this comment

Uh oh!

WillAyd commented Oct 24, 2019

Uh oh!

JasperVanDenBosch commented Oct 24, 2019

Uh oh!

jreback left a comment

Choose a reason for hiding this comment

Uh oh!

jreback Oct 25, 2019

Choose a reason for hiding this comment

Uh oh!

WillAyd commented Oct 25, 2019

Uh oh!

Uh oh!

JasperVanDenBosch commented Jul 31, 2019 •

edited

Loading

JasperVanDenBosch commented Aug 21, 2019 •

edited

Loading

pep8speaks commented Aug 23, 2019 •

edited

Loading