Make scandir an optional dependency

[althonos]

Hi,
I just noticed while trying to install fs on the alpine-python Docker image that it had scandir as a hard requirement. Since scandir is a C extension, this meant that 1. implementations other than CPython couldn't use Pyfilesystem2, and that 2. it required C compilation, so minimal builds needed to have both a C compiler and Python headers installed, both of which can weigh a lot.

With the edits in that PR, the scandir module will be used if it can be found (either scandir is installed or the version is superior to 3.5). In other cases, plain os.listdir module will be used.

[coveralls]

Coverage decreased (-0.5%) to 99.232% when pulling f66b1cc on althonos:feat-purepython into 11b4867 on PyFilesystem:master.

[coveralls]

Coverage decreased (-0.5%) to 99.319% when pulling a2e8d3c on althonos:feat-purepython into 11b4867 on PyFilesystem:master.

[coveralls]

Coverage decreased (-0.4%) to 99.384% when pulling e4be206 on althonos:feat-purepython into 11b4867 on PyFilesystem:master.

[coveralls]

Coverage increased (+0.001%) to 99.78% when pulling 80706d6 on althonos:feat-purepython into 11b4867 on PyFilesystem:master.

[coveralls]

Coverage increased (+0.001%) to 99.78% when pulling 1af4182 on althonos:feat-purepython into 11b4867 on PyFilesystem:master.

[willmcgugan]

Could you cache this os.stat call? It could potentially be called several time. And perhaps you can use the stat structure rather than isdir on L412, because that will do a stat under the hood.

[willmcgugan]

I'd like to suggest a slightly different way of doing the conditional.

try:
    import scandir
except ImportError:
    scandir = None

class OSFS:
    if scandir:
        def _scandir(self, path, namespaces=None, page=None):
            # with scandir implementation
    else:
        def _scandir(self, path, namespaces=None, page=None):
            # without scandir implementation

I think that's a little tidier than having separate _scandir_c and scandir_py functions. What do you think?

[althonos]

At first that's what I wanted to do, but for the sake of testing it's easier to have the two methods accessible at all times, since your way involves reloading the module while mocking an import to be able to access the other implementation. But I'll give it a shot anyway.

[willmcgugan]

I just merges a PR that might conflict with this. You might have to resolve it.

[willmcgugan]

I think I see why you have the 2 functions defined now. I think you can modif tox.ini file to have separate environments for with and without the scandir module. Otherwise its not going to include this in coverage.

[willmcgugan]

Thanks! I concur. Only thing that concerns me is that scandir won't be installed on automatically in environments that it would easily install on. I guess that would just need to be documented.

[althonos]

I'll try to resolve the conflicts shortly.

For the automatic installation of scandir, I tried to search if there was a way to specify "default-but-not-mandatory" requirements, but no luck with that. At some point I thought restricting the installation of scandir to sys_implementation == "cpython", but that would fail for CPython systems without Python headers.

The setup.py could also throw a warning if the version is scandir friendly, like:

# setup.py

import sys
import warnings

if sys.implementation.name == "cpython" and sys.version < "3.5":
    try:
        import scandir
    except ImportError:
        warnings.warn("Consider installing scandir blah blah blah...")

[coveralls]

Coverage increased (+0.001%) to 99.78% when pulling 77e4858 on althonos:feat-purepython into 51b8034 on PyFilesystem:master.

[althonos]

Done, you can merge when you want. I added another environment to tox.ini, and rolled back to defining OSFS._scandir directly depending on the availability of scandir.

[willmcgugan]

Brilliant. Thanks.