<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, May 12, 2013 at 3:04 PM, Ben Hoyt <span dir="ltr"><<a href="mailto:benhoyt@gmail.com" target="_blank">benhoyt@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">> And if we're creating a custom object instead, why return a 2-tuple<br>
> rather than making the entry's name an attribute of the custom object?<br>
><br>
> To me, that suggests a more reasonable API for os.scandir() might be<br>
> for it to be an iterator over "dir_entry" objects:<br>
><br>
> name (as a string)<br>
> is_file()<br>
> is_dir()<br>
> is_link()<br>
> stat()<br>
> cached_stat (None or a stat object)<br>
<br>
</div>Nice! I really like your basic idea of returning a custom object<br>
instead of a 2-tuple. And I agree with Christian that .stat() would be<br>
clearer called .lstat(). I also like your later idea of simply<br>
exposing .dirent (would be None on Windows).<br>
<br>
One tweak I'd suggest is that is_file() etc be called isfile() etc<br>
without the underscore, to match the naming of the <a href="http://os.path.is" target="_blank">os.path.is</a>*<br>
functions.<br>
<div class="im"><br>
> That would actually make sense at an implementation<br>
> level anyway - is_file() etc would check self.cached_lstat first, and<br>
> if that was None they would check self.dirent, and if that was also<br>
> None they would raise an error.<br>
<br>
</div>Hmm, I'm not sure about this at all. Are you suggesting that the<br>
DirEntry object's is* functions would raise an error if both<br>
cached_lstat and dirent were None? Wouldn't it make for a much simpler<br>
API to just call os.lstat() and populate cached_lstat instead? As far<br>
as I'm concerned, that'd be the point of making DirEntry.lstat() a<br>
function.<br>
<br>
In fact, I don't think .cached_lstat should be exposed to the user.<br>
They just call entry.lstat(), and it returns a cached stat or calls<br>
os.lstat() to get the real stat if required (and populates the<br>
internal cached stat value). And the <a href="http://entry.is" target="_blank">entry.is</a>* functions would call<br>
entry.lstat() if dirent was or d_type was DT_UNKNOWN. This would<br>
change relatively nasty code like this:<br>
<br>
files = []<br>
dirs = []<br>
for entry in os.scandir(path):<br>
try:<br>
isdir = entry.isdir()<br>
except NotPresentError:<br>
st = os.lstat(os.path.join(path, <a href="http://entry.name" target="_blank">entry.name</a>))<br>
isdir = stat.S_ISDIR(st)<br>
if isdir:<br>
dirs.append(<a href="http://entry.name" target="_blank">entry.name</a>)<br>
else:<br>
files.append(<a href="http://entry.name" target="_blank">entry.name</a>)<br>
<br>
Into nice clean code like this:<br>
<br>
files = []<br>
dirs = []<br>
for entry in os.scandir(path):<br>
if entry.isfile():<br>
dirs.append(<a href="http://entry.name" target="_blank">entry.name</a>)<br>
else:<br>
files.append(<a href="http://entry.name" target="_blank">entry.name</a>)<br>
<br>
This change would make scandir() usable by ordinary mortals, rather<br>
than just hardcore library implementors.<br>
<br>
In other words, I'm proposing that the DirEntry objects yielded by<br>
scandir() would have .name and .dirent attributes, and .isdir(),<br>
.isfile(), .islink(), .lstat() methods, and look basically like this<br>
(though presumably implemented in C):<br>
<br>
class DirEntry:<br>
def __init__(self, name, dirent, lstat, path='.'):<br>
# User shouldn't need to call this, but called internally by scandir()<br>
<a href="http://self.name" target="_blank">self.name</a> = name<br>
self.dirent = dirent<br>
self._lstat = lstat # non-public attributes<br>
self._path = path<br>
<br>
def lstat(self):<br>
if self._lstat is None:<br>
self._lstat = os.lstat(os.path.join(self._path, <a href="http://self.name" target="_blank">self.name</a>))<br>
return self._lstat<br>
<br>
def isdir(self):<br>
if self.dirent is not None and self.dirent.d_type != DT_UNKNOWN:<br>
return self.dirent.d_type == DT_DIR<br>
else:<br>
return stat.S_ISDIR(self.lstat().st_mode)<br>
<br>
def isfile(self):<br>
if self.dirent is not None and self.dirent.d_type != DT_UNKNOWN:<br>
return self.dirent.d_type == DT_REG<br>
else:<br>
return stat.S_ISREG(self.lstat().st_mode)<br>
<br>
def islink(self):<br>
if self.dirent is not None and self.dirent.d_type != DT_UNKNOWN:<br>
return self.dirent.d_type == DT_LNK<br>
else:<br>
return stat.S_ISLNK(self.lstat().st_mode)<br>
<br>
Oh, and the .dirent would either be None (Windows) or would have<br>
.d_type and .d_ino attributes (Linux, OS X).<br>
<br>
This would make the scandir() API nice and simple to use for callers,<br>
but still expose all the information the OS provides (both the<br>
meaningful fields in dirent, and a full stat on Windows, nicely cached<br>
in the DirEntry object).<br>
<br>
Thoughts?<br></blockquote><div style><br></div><div style>I like the sound of this (which sounds like what you've implemented now though I haven't looked at your code).</div><div style><br></div><div style>-gps</div>
<div style> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
-Ben<br>
</font></span><div class="HOEnZb"><div class="h5">_______________________________________________<br>
Python-Dev mailing list<br>
<a href="mailto:Python-Dev@python.org">Python-Dev@python.org</a><br>
<a href="http://mail.python.org/mailman/listinfo/python-dev" target="_blank">http://mail.python.org/mailman/listinfo/python-dev</a><br>
Unsubscribe: <a href="http://mail.python.org/mailman/options/python-dev/greg%40krypto.org" target="_blank">http://mail.python.org/mailman/options/python-dev/greg%40krypto.org</a><br>
</div></div></blockquote></div><br></div></div>