Skip to content

Stop inheriting from pyarrow.filesystem for pyarrow>=2.0 #411

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

Merged
merged 3 commits into from
Sep 24, 2020
Merged

Stop inheriting from pyarrow.filesystem for pyarrow>=2.0 #411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
96381dd
Stop inheriting from pyarrow.filesystem for pyarrow>=2.0
jorisvandenbossche Sep 17, 2020
8e5075e
move to else clause
jorisvandenbossche Sep 24, 2020
741565f
Merge remote-tracking branch 'upstream/master' into remove-deprecated…
jorisvandenbossche Sep 24, 2020
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
8 changes: 6 additions & 2 deletions fsspec/implementations/tests/test_local.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import pickle
import sys
from contextlib import contextmanager
from distutils.version import LooseVersion
import posixpath
import tempfile

Expand Down Expand Up @@ -325,8 +326,11 @@ def test_get_pyarrow_filesystem():
pa = pytest.importorskip("pyarrow")

fs = LocalFileSystem()
assert isinstance(fs, pa.filesystem.FileSystem)
assert fs._get_pyarrow_filesystem() is fs
if LooseVersion(pa.__version__) < LooseVersion("2.0"):
assert isinstance(fs, pa.filesystem.FileSystem)
assert fs._get_pyarrow_filesystem() is fs
else:
assert not isinstance(fs, pa.filesystem.FileSystem)

class UnknownFileSystem(object):
pass
Expand Down
17 changes: 14 additions & 3 deletions fsspec/spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import logging
import os
import warnings
from distutils.version import LooseVersion
from errno import ESPIPE
from hashlib import sha256
from glob import has_magic
Expand Down Expand Up @@ -70,12 +71,15 @@ def __call__(cls, *args, **kwargs):


try: # optionally derive from pyarrow's FileSystem, if available
# TODO: it should be possible to disable this
import pyarrow as pa

up = pa.filesystem.DaskFileSystem
except ImportError:
up = object
else:
# only derive from the legacy pyarrow's FileSystem for older pyarrow versions
if LooseVersion(pa.__version__) < LooseVersion("2.0"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to use an else clause for this, rather than putting it in the try block, so that the code in the try block is minimized?

up = pa.filesystem.DaskFileSystem
else:
up = object


class AbstractFileSystem(up, metaclass=_Cached):
Expand Down Expand Up @@ -1109,6 +1113,13 @@ def sign(self, path, expiration=100, **kwargs):
"""
raise NotImplementedError("Sign is not implemented for this filesystem")

def _isfilestore(self):
# Originally inherited from pyarrow DaskFileSystem. Keeping this
# here for backwards compatibility as long as pyarrow uses its
# legacy ffspec-compatible filesystems and thus accepts fsspec
# filesystems as well
return False


class AbstractBufferedFile(io.IOBase):
"""Convenient class to derive from to provide buffering
Expand Down