[Python-Dev] Updates to PEP 471, the os.scandir() proposal

[Victor Stinner]

2014-07-09 15:12 GMT+02:00 Ben Hoyt <benhoyt at gmail.com>:
>> Ok, so it means that your example grouping files per type, files and
>> directories, is also wrong. Or at least, it behaves differently than
>> os.walk(). You should put symbolic links to directories in the "dirs"
>> list too.
>>
>> if entry.is_dir():   # is_dir() checks os.lstat()
>>     dirs.append(entry)
>> elif entry.is_symlink() and os.path.isdir(entry):   # isdir() checks os.stat()
>>     dirs.append(entry)
>> else:
>>     non_dirs.append(entry)
>
> Yes, good call. I believe I'm doing this wrong in the scandir.py
> os.walk() implementation too -- hence this open issue:
> https://github.com/benhoyt/scandir/issues/4

The PEP says that DirEntry should mimic pathlib.Path, so I think that
DirEntry.is_dir() should work as os.path.isir(): if the entry is a
symbolic link, you should follow the symlink to get the status of the
linked file with os.stat().

"entry.is_dir() or (entry.is_symlink() and os.path.isdir(entry))"
looks wrong: why would you have to check is_dir() and isdir()?
Duplicating this check is error prone and not convinient.

Pseudo-code:
---
class DirEntry:
    def __init__(self, lstat=None, d_type=None):
        self._stat = None
        self._lstat = lstat
        self._d_type = d_type

    def stat(self):
        if self._stat is None:
            self._stat = os.stat(self.full_name)
        return self._stat

    def lstat(self):
        if self._lstat is None:
            self._lstat = os.lstat(self.full_name)
        return self._lstat

    def is_dir(self):
        if self._d_type is not None:
            if self._d_type == DT_DIR:
                return True
            if self._d_type != DT_LNK:
                return False
        else:
            lstat = self.lstat()
            if stat.S_ISDIR(lstat.st_mode):
                return True
            if not stat.S_ISLNK(lstat.st_mode):
                return False
        stat = self.stat()
        return stat.S_ISDIR(stat.st_mode)
---

DirEntry would be created with lstat (Windows) or d_type (Linux)
prefilled. is_dir() would only need to call os.stat() once for
symbolic links.

With this code, it becomes even more obvious why is_dir() is a method
and not a property ;-)

Victor