BUG: Ignore chartsheets

[ahawryluk]

• closes BUG: AttributeError raised when reading from excel file containing chart sheet #41448
• tests added / passed
• Ensure all linting tests pass, see here for how to run them
• whatsnew entry

Issue #41448 relates specifically to our openpyxl engine, and is resolved by this commit. However, the bug also exists in our pyxlsb engine and cannot be resolved until the upstream willtrnr/pyxlsb#33 is addressed. I propose closing #41448 and leaving the pyxlsb case as an xfail for now. Let me know if there's a better way to track this.

[datapythonista]

Thanks @ahawryluk, looks good. Can you have a look at the CI, and see if you can leave it green please?

[ahawryluk]

@datapythonista I think #41665 tries to address the CI failure on the numpydev build

[ahawryluk]

@datapythonista CI is all green now. Thanks for taking a look at this.

[rhshadrach]

Changes look good - I wonder if we should be changing the documentation for the sheet_names argument:

Strings are used for sheet names. Integers are used in zero-indexed sheet positions. Lists of strings/integers are used to request multiple sheets. Specify None to get all sheets.

Get all worksheets instead?

Also, what happens if the index specified by sheetnames is a chart-sheet? What is a previous sheet is a chart sheet - is the chart sheet counted in the indexing?

[ahawryluk]

Hi @rhshadrach, thanks for the questions. I've attempted to address them in the docstring without getting too wordy. String and integer inputs are interpreted with respect to ExcelFile.sheet_names, which now excludes chart sheets, so the accepted inputs should be internally consistent and the default sheet_name=0 won't crash if there's a chart sheet in the first position.

[rhshadrach]

Thanks @ahawryluk - makes sense. One last question - what is the error raised if the sheet name (as a string) is a chart sheet? If it's sensible, can you add a test for this.

[ahawryluk]

@rhshadrach Good idea. It raises the same ValueError you would get for an invalid sheet name, so I've added the tests for loading by str or by an int out of range.

[rhshadrach]

lgtm - can you resolve conflict and move whatsnew note to 1.4

[rhshadrach]

lgtm

[rhshadrach]

Thanks @ahawryluk!