-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: pathlib.Path in io #16292
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
BUG: pathlib.Path in io #16292
Conversation
pandas/tests/io/test_pickle.py
Outdated
@@ -299,6 +300,15 @@ def test_pickle_v0_15_2(): | |||
tm.assert_categorical_equal(cat, pd.read_pickle(pickle_path)) | |||
|
|||
|
|||
def test_pickle_path_pathlib(): | |||
tm._skip_if_no_pathlib() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of making a specific test can u add a general routine like we have for round_trip_pickle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and then use it here
ideally add tests for all of the io functions (and xfail id they don't work)
and test for localpath as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, let me know if what I just pushed is what you had in mind and I'll add to other io functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep exactly like this!
maybe put a little docs in the testing file above these 3 tests routines.
pandas/tests/io/test_pickle.py
Outdated
|
||
|
||
def test_pickle_path_localpath(): | ||
tm._skip_if_no_localpath() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move the skip routines to the actual test functions themselves (in util/testing)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ignore this, you can just use a skip importer below.
pandas/util/testing.py
Outdated
|
||
|
||
def round_trip_localpath(writer, reader, path=None): | ||
from py.path import local as LocalPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g. here.
and you need an pytimportor.skip here (for both of these).
Codecov Report
@@ Coverage Diff @@
## master #16292 +/- ##
==========================================
- Coverage 90.39% 90.37% -0.02%
==========================================
Files 161 161
Lines 50863 50880 +17
==========================================
+ Hits 45977 45985 +8
- Misses 4886 4895 +9
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #16292 +/- ##
==========================================
- Coverage 90.39% 90.36% -0.04%
==========================================
Files 161 161
Lines 50863 50885 +22
==========================================
+ Hits 45977 45981 +4
- Misses 4886 4904 +18
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fantastic. thanks! (tiny comments). merge when ready.
pandas/io/common.py
Outdated
@@ -314,6 +314,8 @@ def _get_handle(path_or_buf, mode, encoding=None, compression=None, | |||
|
|||
handles = list() | |||
f = path_or_buf | |||
# Convert pathlib.Path/py.path.local or string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a blank line
doc/source/whatsnew/v0.20.2.txt
Outdated
@@ -33,6 +33,8 @@ Performance Improvements | |||
Bug Fixes | |||
~~~~~~~~~ | |||
|
|||
- Bug in using ``pathlib.Path`` object with io functions (:issue:`16291`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
localpath too?
pandas/tests/io/parser/common.py
Outdated
|
||
def test_pickle_path_localpath(self): | ||
df = tm.makeDataFrame() | ||
result = tm.round_trip_pathlib(df.to_csv, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
round_trip_localpath ?
for those ones which xfail this, can you create an issue (put all in issue). Though might be one that just needs updating. |
pandas/tests/io/sas/test_sas7bdat.py
Outdated
@@ -65,6 +65,26 @@ def test_from_iterator(self): | |||
tm.assert_frame_equal(df, df0.iloc[2:5, :]) | |||
rdr.close() | |||
|
|||
def test_path_pathlib(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like needs xfailing here
can you add tests for feather-format as well (it should work I think, not sure) |
There was a failure in one job: https://travis-ci.org/pandas-dev/pandas/jobs/231516184#L2140 but that looks unrelated. |
thanks! |
* BUG: pathlib.Path in io * CLN: factor out pathlib roundtrip * add localpath tests for other io * fixup * xfail SAS; type in parser * missing import * xfail for pandas-dev#14704 * fix to_csv * lint * lint cleanup * add feather (xfail)
* BUG: pathlib.Path in io * CLN: factor out pathlib roundtrip * add localpath tests for other io * fixup * xfail SAS; type in parser * missing import * xfail for pandas-dev#14704 * fix to_csv * lint * lint cleanup * add feather (xfail) (cherry picked from commit 4cd8458)
* BUG: pathlib.Path in io * CLN: factor out pathlib roundtrip * add localpath tests for other io * fixup * xfail SAS; type in parser * missing import * xfail for pandas-dev#14704 * fix to_csv * lint * lint cleanup * add feather (xfail)
git diff upstream/master --name-only -- '*.py' | flake8 --diff