io/msgpack: Raise a more informative exception on file not found #27162
Conversation
pandas/tests/io/test_packers.py
Outdated
| @@ -934,3 +934,7 @@ def test_msgpack_period_freq(self): | |||
| s = Series(np.random.rand(5), index=date_range('20130101', periods=5)) | |||
| r = read_msgpack(s.to_msgpack()) | |||
| repr(r) | |||
|
|
|||
| def test_msgpack_file_not_found(self): | |||
| bad_path = '1/2/3/4/path/does/not/exist' | |||
There was a problem hiding this comment.
can you add the issue number here as a comment
There was a problem hiding this comment.
I removed this test and added the issue number inline where the fix was done.
pandas/tests/io/test_packers.py
Outdated
|
|
||
| def test_msgpack_file_not_found(self): | ||
| bad_path = '1/2/3/4/path/does/not/exist' | ||
| pytest.raises(read_msgpack, FileNotFoundError, bad_path) |
There was a problem hiding this comment.
use context manager or better still, i don't think you need to add a test.
changing TestCommonIOCapabilities.test_read_non_existant and TestCommonIOCapabilities.test_read_expands_user_home_dir in pandas/tests/io/test_common.py would be sufficient.
There was a problem hiding this comment.
Ah, nice - looks like these tests handle this behavior already.
pandas/io/packers.py
Outdated
| @@ -184,6 +184,8 @@ def read(fh): | |||
| if exists: | |||
| with open(path_or_buf, 'rb') as fh: | |||
| return read(fh) | |||
| else: | |||
| return FileNotFoundError('{} not found'.format(path_or_buf)) | |||
There was a problem hiding this comment.
although this may simplify testing, i'd be inclined to just let Python raise the system specific exception.
i.e.
replace
# see if we have an actual file
if isinstance(path_or_buf, str):
try:
exists = os.path.exists(path_or_buf)
except (TypeError, ValueError):
exists = False
if exists:
with open(path_or_buf, 'rb') as fh:
return read(fh)with
if isinstance(path_or_buf, str):
with open(path_or_buf, 'rb') as fh:
return read(fh)and it raises (on windows)..
FileNotFoundError: [Errno 2] No such file or directory: 'this/path/does/not/exist'
simonjayhawkins
added
the
Error Reporting
As opposed to checking if it exists first.
|
Okay, requested fixes should be in place now:
|
|
@smspillaz looks like there are some merge conflicts - can you clean those up? |
|
Can be closed as #27201 was merged which fixes the same issue |
git diff upstream/master -u -- "*.py" | flake8 --diff