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.

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

Support for Path objects in io packages #4606

Merged
merged 7 commits into from Mar 1, 2016
15 changes: 15 additions & 0 deletions astropy/io/ascii/tests/test_read.py
Expand Up @@ -26,6 +26,12 @@
else:
HAS_BZ2 = True

try:
import pathlib
except ImportError:
HAS_PATHLIB = False
else:
HAS_PATHLIB = True

@pytest.mark.parametrize('fast_reader', [True, False, 'force'])
def test_convert_overflow(fast_reader):
Expand Down Expand Up @@ -1092,3 +1098,12 @@ def test_table_with_no_newline():
t = ascii.read(table, **kwargs)
assert t.colnames == ['a', 'b']
assert len(t) == 0

@pytest.mark.skipif('not HAS_PATHLIB')
def test_path_object():
fpath = pathlib.Path('t/simple.txt')
data = ascii.read(fpath)

assert len(data) == 2
assert sorted(list(data.columns)) == ['test 1a', 'test2', 'test3', 'test4']
assert data['test2'][1] == 'hat2'
6 changes: 3 additions & 3 deletions astropy/io/ascii/ui.py
Expand Up @@ -186,9 +186,9 @@ def read(table, guess=None, **kwargs):

Parameters
----------
table : str, file-like, list
Input table as a file name, file-like object, list of strings, or
single newline-separated string.
table : str, file-like, list, pathlib.Path object
Input table as a file name, file-like object, list of strings,
single newline-separated string or pathlib.Path object .
guess : bool
Try to guess the table format (default= ``True``)
format : str, `~astropy.io.ascii.BaseReader`
Expand Down
12 changes: 11 additions & 1 deletion astropy/io/fits/file.py
Expand Up @@ -19,7 +19,7 @@
from .util import (isreadable, iswritable, isfile, fileobj_open, fileobj_name,
fileobj_closed, fileobj_mode, _array_from_file,
_array_to_file, _write_string)
from ...extern.six import b, string_types
from ...extern.six import b, string_types, PY3
from ...utils.data import download_file, _is_url
from ...utils.decorators import classproperty
from ...utils.exceptions import AstropyUserWarning
Expand Down Expand Up @@ -74,6 +74,13 @@
PKZIP_MAGIC = b('\x50\x4b\x03\x04')
BZIP2_MAGIC = b('\x42\x5a')

try:
from pathlib import Path
Copy link
Member

Choose a reason for hiding this comment

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

Make this import pathlib for consistency with usage in the other modules.

except:
HAS_PATHLIB = False
else:
HAS_PATHLIB = True

class _File(object):
"""
Represents a FITS file on disk (or in some other file-like object).
Expand All @@ -97,6 +104,9 @@ def __init__(self, fileobj=None, mode=None, memmap=None, clobber=False,
return
else:
self.simulateonly = False
# If fileobj is of type pathlib.Path
if PY3 and HAS_PATHLIB and isinstance(fileobj, Path):
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to check PY3, all that matters is HAS_PATHLIB to make sure the second logical test will execute properly.

fileobj = str(fileobj)

# Holds mmap instance for files that use mmap
self._mmap = None
Expand Down
2 changes: 1 addition & 1 deletion astropy/io/fits/hdu/hdulist.py
Expand Up @@ -28,7 +28,7 @@ def fitsopen(name, mode='readonly', memmap=None, save_backup=False,

Parameters
----------
name : file path, file object or file-like object
name : file path, file object, file-like object or pathlib.Path object
File to be opened.

mode : str, optional
Expand Down
24 changes: 24 additions & 0 deletions astropy/io/fits/tests/test_core.py
Expand Up @@ -20,11 +20,19 @@
from ....io import fits
from ....tests.helper import pytest, raises, catch_warnings, ignore_warnings
from ....utils.exceptions import AstropyDeprecationWarning
from ....utils.data import get_pkg_data_filename
from . import FitsTestCase
from ..convenience import _getext
from ..diff import FITSDiff
from ..file import _File, GZIP_MAGIC

try:
import pathlib
except ImportError:
HAS_PATHLIB = False
else:
HAS_PATHLIB = True


class TestCore(FitsTestCase):
def test_with_statement(self):
Expand Down Expand Up @@ -65,6 +73,22 @@ def test_byteswap(self):
with fits.open(self.temp('test.fits')) as p:
assert p[1].data[1]['foo'] == 60000.0

@pytest.mark.skipif('not HAS_PATHLIB')
def test_fits_file_path_object(self):
"""
Testing when fits file is passed as pathlib.Path object #4412.
"""
fpath = pathlib.Path(get_pkg_data_filename('data/tdim.fits'))
hdulist = fits.open(fpath)

assert hdulist[0].filebytes() == 2880
assert hdulist[1].filebytes() == 5760

hdulist2 = fits.open(self.data('tdim.fits'))

assert FITSDiff(hdulist2, hdulist).identical is True


def test_add_del_columns(self):
p = fits.ColDefs([])
p.add_col(fits.Column(name='FOO', format='3J'))
Expand Down
3 changes: 2 additions & 1 deletion astropy/io/votable/table.py
Expand Up @@ -196,7 +196,8 @@ def validate(source, output=None, xmllint=False, filename=None):
Parameters
----------
source : str or readable file-like object
Path to a VOTABLE_ xml file.
Path to a VOTABLE_ xml file or pathlib.path
object having Path to a VOTABLE_ xml file.

output : writable file-like object, optional
Where to output the report. Defaults to ``sys.stdout``.
Expand Down
19 changes: 19 additions & 0 deletions astropy/io/votable/tests/table_test.py
Expand Up @@ -13,6 +13,14 @@
from ....utils.data import get_pkg_data_filename, get_pkg_data_fileobj
from ..table import parse, writeto
from .. import tree
from ....tests.helper import pytest

try:
import pathlib
except ImportError:
HAS_PATHLIB = False
else:
HAS_PATHLIB = True


def test_table(tmpdir):
Expand Down Expand Up @@ -118,6 +126,17 @@ def test_table_read_with_unnamed_tables():

assert len(t) == 1

@pytest.mark.skipif('not HAS_PATHLIB')
def test_votable_path_object():
"""
Testing when votable is passed as pathlib.Path object #4412.
"""
fpath = pathlib.Path(get_pkg_data_filename('data/names.xml'))
table = parse(fpath).get_first_table().to_table()

assert len(table) == 1
assert int(table[0][3]) == 266


def test_from_table_without_mask():
from ....table import Table, Column
Expand Down
46 changes: 46 additions & 0 deletions astropy/io/votable/tests/vo_test.py
Expand Up @@ -29,6 +29,13 @@
from ....utils.data import get_pkg_data_filename, get_pkg_data_filenames
from ....tests.helper import pytest, raises, catch_warnings

try:
import pathlib
except ImportError:
HAS_PATHLIB = False
else:
HAS_PATHLIB = True

# Determine the kind of float formatting in this build of Python
if hasattr(sys, 'float_repr_style'):
legacy_float_repr = (sys.float_repr_style == 'legacy')
Expand Down Expand Up @@ -820,6 +827,45 @@ def test_validate():
assert truth == output


@pytest.mark.skipif('not HAS_PATHLIB')
def test_validate_path_object():
"""
Validating when source is passed as path object. (#4412)
Creating different method from ``test_validate`` so that it could be
Copy link
Member

Choose a reason for hiding this comment

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

I think you can just add an argument to test_validate, e.g.

def test_validate(test_path_object=False):
    fpath = get_pkg_data_filename('data/regression.xml')
    if test_path_object:
        fpath = pathlib.Path(fpath)
    ...

Then you can still have a separate test_validate_path_object() (which will be skipped for PY2) but just calls test_validate(test_path_object=True).

skipped if ``pathlib`` isnt available without affecting ``test_validate``
"""
output = io.StringIO()
fpath = pathlib.Path(get_pkg_data_filename('data/regression.xml'))

with catch_warnings():
result = validate(fpath,
output, xmllint=False)

assert result == False

output.seek(0)
output = output.readlines()

with io.open(
get_pkg_data_filename('data/validation.txt'),
'rt', encoding='utf-8') as fd:
truth = fd.readlines()

truth = truth[1:]
output = output[1:-1]

for line in difflib.unified_diff(truth, output):
if six.PY3:
sys.stdout.write(
line.replace('\\n', '\n'))
else:
sys.stdout.write(
line.encode('unicode_escape').
replace('\\n', '\n'))

assert truth == output


def test_gzip_filehandles(tmpdir):
votable = parse(
get_pkg_data_filename('data/regression.xml'),
Expand Down
16 changes: 14 additions & 2 deletions astropy/utils/data.py
Expand Up @@ -106,7 +106,7 @@ def _is_inside(path, parent_path):
def get_readable_fileobj(name_or_obj, encoding=None, cache=False,
show_progress=True, remote_timeout=None):
"""
Given a filename or a readable file-like object, return a context
Given a filename, pathlib.Path object or a readable file-like object, return a context
manager that yields a readable file-like object.

This supports passing filenames, URLs, and readable file-like objects,
Expand Down Expand Up @@ -167,6 +167,14 @@ def get_readable_fileobj(name_or_obj, encoding=None, cache=False,
# passed in. In that case it is not the responsibility of this
# function to close it: doing so could result in a "double close"
# and an "invalid file descriptor" exception.
PATH_TYPES = six.string_types
try:
from pathlib import Path
Copy link
Member

Choose a reason for hiding this comment

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

Better to follow the same idiom from testing and define HAS_PATHLIB. This allows for the possibility of a Python 4 that may or may not have pathlib.

Note: you should change my original implementation in io.registry to match (#4405). I fell into the mistake of testing for PY3. See http://astrofrog.github.io/blog/2016/01/12/stop-writing-python-4-incompatible-code/. In fact the fix to io.registry might not be needed now that you are patching get_readable_fileobj. I haven't looked through the logic that closely but you should look and test.

except:
pass
else:
PATH_TYPES += (Path,)

close_fds = []
delete_fds = []

Expand All @@ -175,7 +183,11 @@ def get_readable_fileobj(name_or_obj, encoding=None, cache=False,
remote_timeout = conf.remote_timeout

# Get a file object to the content
if isinstance(name_or_obj, six.string_types):
if isinstance(name_or_obj, PATH_TYPES):
# name_or_obj could be a Path object in Python 3
if six.PY3:
Copy link
Member

Choose a reason for hiding this comment

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

Notice that this code becomes easier to understand:

        if HAS_PATHLIB:
            ...

This avoids one level of contextual knowledge that only Python 3 has pathlib. So then the comment would be more like # name_or_obj could be a Path object if pathlib is available.

name_or_obj = str(name_or_obj)

is_url = _is_url(name_or_obj)
if is_url:
name_or_obj = download_file(
Expand Down
13 changes: 13 additions & 0 deletions astropy/utils/tests/test_data.py
Expand Up @@ -42,6 +42,13 @@
else:
HAS_XZ = True

try:
import pathlib
except ImportError:
HAS_PATHLIB = False
else:
HAS_PATHLIB = True

@remote_data
def test_download_nocache():
from ..data import download_file
Expand Down Expand Up @@ -396,3 +403,9 @@ def test_get_readable_fileobj_cleans_up_temporary_files(tmpdir, monkeypatch):
# Assert that the temporary file was empty after get_readable_fileobj()
# context manager finished running
assert len(tempdir_listing) == 0

@pytest.mark.skipif('not HAS_PATHLIB')
def test_path_objects_get_readable_fileobj():
fpath = pathlib.Path(get_pkg_data_filename(os.path.join('data', 'local.dat')))
with get_readable_fileobj(fpath) as f:
assert f.read().rstrip() == 'This file is used in the test_local_data_* testing functions\nCONTENT'