Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add capability to handle Path/LocalPath objects #11051

Closed
wants to merge 1 commit into from
Closed

Add capability to handle Path/LocalPath objects #11051

wants to merge 1 commit into from

Conversation

flying-sheep
Copy link
Contributor

fixes #11033

@jreback
Copy link
Contributor

jreback commented Sep 10, 2015

ok, looks reasonable

the pathlib test should pass on 3.4 and skip on all others (pls confirm via the travis log)
however, pls add py to the requirements-2.7.txt so conda will install it (so it will test)

jreback
jreback reviewed Sep 10, 2015
View reviewed changes
pandas/io/common.py
@@ -118,6 +118,33 @@ def _expand_user(filepath_or_buffer):
return filepath_or_buffer


def _stringify_path(filepath_or_buffer):
Copy link
Contributor

Choose a reason for hiding this comment

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

where is this actuallly called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops, didn’t add that line before i pushed. it’s being called now

@jreback jreback added Enhancement IO Data IO issues that don't fit into a more specific label Compat pandas objects compatability with Numpy or Python functions labels Sep 10, 2015
@TomAugspurger
Copy link
Contributor

Docs would be great as well, in the docstring and in the doc/source/io.rst.

jreback
jreback reviewed Sep 20, 2015
View reviewed changes
pandas/io/tests/test_common.py
import pandas.util.testing as tm

from pandas.io import common

try:
import pathlib
PATHLIB_INSTALLED = True
Copy link
Contributor

Choose a reason for hiding this comment

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

make these into functions that raise nose.SkipTest (e.g. put them in this module, but see how this is done in utils.testing, e.g. the _skip_if......

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@jreback jreback added this to the 0.17.1 milestone Sep 20, 2015
@jreback
Copy link
Contributor

jreback commented Oct 5, 2015

can you add pathlib to one of the py2 builds
pls add a whatsnew note for 0.17.1

jreback
jreback reviewed Oct 5, 2015
View reviewed changes
doc/source/whatsnew/v0.17.1.txt
@@ -23,6 +23,10 @@ Enhancements
Other Enhancements
^^^^^^^^^^^^^^^^^^

- :func:`~pandas.io.parsers.read_csv` and :func:`~pandas.io.parsers.read_table` now
also accept :class:`python:pathlib.Path`, or :class:`py:py._path.local.LocalPath`
as first argument.
Copy link
Contributor

Choose a reason for hiding this comment

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

say for the filepath_or_buffer argument

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@flying-sheep
Copy link
Contributor Author

all done!

jreback
jreback reviewed Oct 5, 2015
View reviewed changes
ci/requirements-2.7.run
@@ -19,3 +19,4 @@ pymysql=0.6.3
html5lib=1.0b2
beautiful-soup=4.2.1
statsmodels
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you have add py here as well (otherwise these will skip). Note on the 2.6 build these SHOULD skip.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@flying-sheep
Copy link
Contributor Author

anaconda can’t install pathlib. wat do?

@jreback
Copy link
Contributor

jreback commented Oct 6, 2015

add to ci/requirements-2.7.pip

jreback
jreback reviewed Oct 6, 2015
View reviewed changes
ci/requirements-2.7.run
@@ -19,3 +19,5 @@ pymysql=0.6.3
html5lib=1.0b2
beautiful-soup=4.2.1
statsmodels
pathlib
py
Copy link
Contributor

Choose a reason for hiding this comment

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

add these to requirements-2.7.pip (instead of here)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, done

@jreback
Copy link
Contributor

jreback commented Oct 8, 2015

pls squash. otherwise lgtm.

@flying-sheep
Copy link
Contributor Author

squashed 👍

jreback
jreback reviewed Oct 9, 2015
View reviewed changes
doc/source/whatsnew/v0.17.1.txt
@@ -23,6 +23,10 @@ Enhancements
Other Enhancements
^^^^^^^^^^^^^^^^^^

- :func:`~pandas.io.parsers.read_csv` and :func:`~pandas.io.parsers.read_table` now
also accept :class:`python:pathlib.Path`, or :class:`py:py._path.local.LocalPath`
for the ``filepath_or_buffer`` argument.
Copy link
Contributor

Choose a reason for hiding this comment

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

add the issue number here (or the PR number IIRC this doesn't have an issue)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

jreback
jreback reviewed Oct 11, 2015
View reviewed changes
pandas/io/common.py


try:
from py.path.local import LocalPath
Copy link
Contributor

Choose a reason for hiding this comment

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

This is broken, you are skipping this test when it should work because its not imported correctly

In [1]: import py

In [2]: py.path.local.LocalPath
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-2-ac7907368c6c> in <module>()
----> 1 py.path.local.LocalPath

AttributeError: type object 'LocalPath' has no attribute 'LocalPath'

In [3]: py.path.local          
Out[3]: py._path.local.LocalPath

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops, fixed. shoulda tested it 😆

jreback
jreback reviewed Oct 12, 2015
View reviewed changes
pandas/io/common.py


try:
from py.path import local
Copy link
Contributor

Choose a reason for hiding this comment

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

I like your naming import before, e.g.

from py.path import local as LocalPath

@flying-sheep
Copy link
Contributor Author

done, sorry for the holdup

@jreback
Copy link
Contributor

jreback commented Oct 17, 2015

merged via 82f0033

thanks!

@jreback jreback closed this Oct 17, 2015
@jreback jreback mentioned this pull request Oct 17, 2015
Closed
@flying-sheep flying-sheep deleted the pathlib branch October 17, 2015 14:45
@flying-sheep
Copy link
Contributor Author

🎆 yeah!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Compat pandas objects compatability with Numpy or Python functions Enhancement IO Data IO issues that don't fit into a more specific label
Projects
None yet
Milestone
0.17.1
Development

Successfully merging this pull request may close these issues.

accept pathlib paths wherever a path/file is expected
3 participants
@flying-sheep @jreback @TomAugspurger