Skip to content

replace KeyError with LookupError in the types of possible read_hdf errors #52783

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.

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

Merged
merged 6 commits into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v2.1.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ I/O
^^^
- :meth:`DataFrame.to_orc` now raising ``ValueError`` when non-default :class:`Index` is given (:issue:`51828`)
- :meth:`DataFrame.to_sql` now raising ``ValueError`` when the name param is left empty while using SQLAlchemy to connect (:issue:`52675`)
- Bug in :func:`read_hdf` not properly closing store after a ``IndexError`` is raised (:issue:`52781`)
- Bug in :func:`read_html`, style elements were read into DataFrames (:issue:`52197`)
- Bug in :func:`read_html`, tail texts were removed together with elements containing ``display:none`` style (:issue:`51629`)
-
Expand Down
2 changes: 1 addition & 1 deletion pandas/io/pytables.py
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ def read_hdf(
chunksize=chunksize,
auto_close=auto_close,
)
except (ValueError, TypeError, KeyError):
except (ValueError, TypeError, LookupError):
if not isinstance(path_or_buf, HDFStore):
# if there is an error, close the store if we opened it.
with suppress(AttributeError):
Expand Down
14 changes: 14 additions & 0 deletions pandas/tests/io/pytables/test_read.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,20 @@ def test_read_missing_key_close_store(tmp_path, setup_path):
df.to_hdf(path, "k2")


def test_read_index_error_close_store(tmp_path, setup_path):
# GH 25766
path = tmp_path / setup_path
df = DataFrame({"A": [], "B": []}, index=[])
df.to_hdf(path, "k1")

with pytest.raises(IndexError, match=r"list index out of range"):
read_hdf(path, "k1", stop=0)

# smoke test to test that file is properly closed after
# read with IndexError before another write
df.to_hdf(path, "k1")
Comment on lines +54 to +56
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# smoke test to test that file is properly closed after
# read with IndexError before another write
df.to_hdf(path, "k1")
# smoke test to test that file is properly closed after
# read with IndexError before another write
with tm.assert_produces_warning(None):
df.to_hdf(path, "k1")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why would this produce a warning? If the file is already open an exception is raised regardless of warning filter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change would need to be in test_read_missing_key_opened_store also

Copy link
Member

Choose a reason for hiding this comment

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

This is just to help assert something additional with this call (that no warning is raised)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's correct to do that here



def test_read_missing_key_opened_store(tmp_path, setup_path):
# GH 28699
path = tmp_path / setup_path
Expand Down