ENH: Add sheet name to error message when reading Excel #49186
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When reading Excel files with `BaseExcelReader`, is possible that `TextParser` raises `ValueError` on different situations. As the reader works on a sheet basis, including the name of the sheet that caused the exception can be useful for debugging. The implemented solution captures any `ValueError` that gets raised while parsing and reading a sheet from an Excel file, then adds the name of the sheet to the exception message and "re-raises" the exception using the `Exception.with_traceback` method. Why use `with_traceback`? This allow-us to maintin the focus on the actual line that raised the ValueError instead of directing the attention to the introduced block of code. Signed-off-by: Antonio Ossa Guerra <[email protected]>
The test performs a `read_excel` operation on a blank Excel file while also passing `[1]` as header value. This operations results in a `ValueError`. Before the fix, the error message did not display any information about the offending sheet, making it harder to debug Excel files with several sheets. For this reason, the value of `sheet_name` was explicitly set to its default value (`None`). The test compares the `ValueError` message with the expected message that contains the offending sheet name. Signed-off-by: Antonio Ossa Guerra <[email protected]>
mroeschke
added
Error Reporting
aaossa
changed the title
The introduced enhancement of the error message on `read_excel` is now included in the whatsnew Signed-off-by: Antonio Ossa Guerra <[email protected]>
3 tasks
phofl
reviewed
Oct 20, 2022
rhshadrach
reviewed
Oct 23, 2022
Instead of handling traceback of the capture exception manually, this simpler syntax looks better, is clearer, and produces the same result Signed-off-by: Antonio Ossa Guerra <[email protected]>
Instead of including the sheet name on the traceback when a ValueError is raised, we'll do it for any kind of Exception. In particular, this is because some of the arguments for `read_excel` are callables, and they can raise any Exception. Including the sheet name might be helpful in some of those cases, so we'll capture them too Signed-off-by: Antonio Ossa Guerra <[email protected]>
The previous message only included `ValueError` as the messages that displayed the sheet name on their traceback, but that behavior is now present on any exception Signed-off-by: Antonio Ossa Guerra <[email protected]>
aaossa
changed the title
aaossa
requested review from
phofl and
rhshadrach
and removed request for
phofl and
rhshadrach
mroeschke
approved these changes
Oct 24, 2022
phofl
approved these changes
Oct 24, 2022
|
thx @aaossa |
noatamir
pushed a commit
to noatamir/pandas
that referenced
this pull request
Nov 9, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added type annotations to new arguments/methods/functions.doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.Description of changes
When reading Excel files with
BaseExcelReader, is possible thatTextParserraises anExceptionon different situations. As the reader works on a sheet basis, including the name of the sheet that caused the exception can be useful for debugging.The implemented solution captures any
Exceptionthat gets raised while parsing and reading a sheet from an Excel file, then adds the name of the sheet to the exception message and "re-raises" the exception.