[Python-ideas] PEP: Extended stat_result (First Draft)

[Pieter Nagel]

Comments inline,

On Mon, 2013-05-06 at 11:31 +0200, Charles-François Natali wrote:
> Hello,
> 
> Looks good, a couple remarks.
> 
> > Added methods on ``stat_result``
> > --------------------------------
> 
> There are too many is_XXX methods.
> 
> > is_dir()
> >     Equivalent to ``bool(stat.S_ISDIR(self.st_mode))``.
> > is_file()
> >     Equivalent to ``bool(stat.S_ISREG(self.st_mode))``.
> > is_symbolic_link()
> >     Equivalent to ``bool(stat.S_ISLNK(self.st_mode))``.
> 
> OK.
> 
> > is_character_device()
> >     Equivalent to ``bool(stat.S_ISCHR(self.st_mode))``.
> >
> > is_block_device()
> >     Equivalent to ``bool(stat.S_ISBLK(self.st_mode))``.
> >
> > is_fifo()
> >     Equivalent to ``bool(stat.S_ISFIFO(self.st_mode))``.
> >
> > is_socket()
> >     Equivalent to ``bool(stat.S_ISSOCK(self.st_mode)``.
> 
> Those are IMO useless.
> If we go down this road, we can also add Solaris door files, and
> another bazillion files types (see the previous thread).

I agree that adding methods like is_socket(), which are already
Unix-specific, will create precedent and pressure to add concepts like
Solaris door files here as well.

And I agree that decisions taken here will have to be consistent with
what we want for these platform-specific concepts as well.

But I am not so sure if adding the concepts on stat_result is
necessarily such a bad thing.

It seems we will soon have the precedent of PEP 428, which sequesters
the platform-specific concepts into separate classes such as PosixPath
and NTPath.

Perhaps one can similarly manage the potential future explosion of
platform-specific is_door() etc. methods you are concerned about, by
implementing stat_result using different concrete types, like
solaris_stat_result and nt_st_result and the like?

> Something like is_other() or is_special() is enough. Code needing more
> specific information about the file type know how to use S_XXX, and
> has to be non portable anyway.

But note that if Python is going to handle Solaris doors etc. at all,
the concept will have to be added *somewhere*. I am not sure why letting
stat.S_XXX functions accumulate is so much better than allowing
stat_result.is_xxx() methods accumulate.

> I think it could be better to add a "key" attribute, which would
> return (st_dev, st_ino). Then, checking that two stat results refer to
> the same file is simply a matter of comparing their key.

Good idea. This will allow more to be done with stat keys than just
comparing them for equality.

> Could the following be properties?
> 
> > permission_bits()
> >     This shall return ``stat.S_IMODE(self.st_mode)``.
> 
> good
> 
> > file_mode()
> >     This shall return ``stat.filemode(stat.S_IMODE(self.st_mode))``, i.e. a
> >     string of the form ‘-rwxrwxrwx’.
> 
> Interesting.
> I don't like the name, though.

Note that the only reason I proposed this is because the functionality
already exists in the stat module as stat.filemode(), which is also what
I based the name on.

> Also, if we provide a way to return a string representation from the
> permission bits, we also probably want an helper to do it the other
> way around, i.e. a permission bit array from a string.
> So I think those two methods we be better as static helper methods.

I agree that adding the converse functionality would be symmetrical, but
AFAIK, unlike filemode() that functionality does not already exist in
the stdlib.

I suppose in the end it depends on how much I want to merely regularise
existing features in the stdlib, versus expanding functionality out to
make a more cohesive whole.

I'll take this into account.

> > format()
> >     This shall return ``stat.S_IFMT(self.st_mode)``.
> 
> Is this really necessary?
> AFAICT, S_IFMT is only useful as an helper for S_ISREG/etc. I don't
> see any added value in exposing it.

I was guided (maybe too much?) by a desire to have all functionality on
the stat module available without importing stat. But you convinced me
it is not needed.

> > Added functions in ``os.path``
> > ------------------------------
> >
> > is_dir(f)
> >     This shall be an alias for the existing isdir(f).
> 
> Why?

That depends on the open question "how important is it that new stdlib
code be completely PEP-8 compliant", as well as "how important is it
that the same concepts in os.path have the same name as they will have
on the statr_result object". I'm very keen on feedback on the latter.

> 
> > is_character_device(f)
> >     This shall return ``os.stat(f).is_character_device()``, or ``False`` if
> >     ``f`` does not exist.
> >
> > is_block_device(f)
> >     This shall return ``os.stat(f).is_block_device()``, or ``False`` if
> >     ``f`` does not exist.
> >
> > is_file()
> >     This shall be an alias for the existing isfile(f).
> >
> > is_fifo()
> >     This shall return ``os.stat(f).is_fifo()``, or ``False`` if
> >     ``f`` does not exist.
> >
> > is_symbolic_link()
> >     This shall return ``os.stat(f).is_symbolic_link()``, or ``False`` if
> >     ``f`` does not exist.
> >
> > is_socket()
> >     This shall return ``os.stat(f).is_socket()``, or ``False`` if
> >     ``f`` does not exist.
> 
> Same remark as above, I'm not convinced that all those special cases
> are necessary.
> 
> > Rationale
> > =========
> >
> > The PEP is strongly motivated by a desire for symmetry between functions in
> > ``os.path`` and methods on ``stat_result``.
> >
> > Therefore, for each predicate function in ``os.path`` that is essentially
> > just an interrogation of ``os.*stat()``, given an existing path, the
> > similarly-named predicate method on ``stat_result`` should have the exact
> > same semantics.
> >
> > This definition does not cover the case where the path being interrogated
> > does not exist.  In those cases, predicate functions in ``os.path``, such
> > as ``os.path.isfile()``, will return ``False``, whereas ``os.*stat()`` will
> > raise FileNotFoundError even before any ``stat_result`` is returned that
> > could have been interrogated.  This renders considerations of how the
> > proposed new predicates on ``stat_result`` could have been symmetrical with
> > functions in ``os.path``, if their ``stat_result`` had existed, moot, and
> > this PEP does not propose doing anything about the situation (but see `Open
> > Issues`_ below).
> >
> > Secondly, this definition refers to ‘similarly-named’ predicates instead of
> > ‘identically-named’ predicates, because the names in ``os.path`` pre-date
> > PEP 8 [#PEP-8]_, and are not compliant with it.  This PEP takes the
> > position that it is better that the new predicate methods on
> > ``stat_result`` be named in compliance with PEP 8 [#PEP-8]_ (i.e.
> > ``is_file()``), than that they be precisely identical to the names in
> > ``os.path`` (i.e ``isfile()``).  Note also that PEP 428 [#PEP-428]_ also
> > specifies PEP-8 compliant names such as ``is_file()`` for the exact same
> > concepts, and if PEP 428 [#PEP-428]_ should be accepted, the issue would be
> > even more pertinent.
> >
> > Lastly, this PEP takes the notion of symmetry as far as adding methods and
> > aliases to the existing ``os.path`` in order to be symmetrical with the
> > added behaviour on ``stat_result``.  But the author is least strongly
> > convicted of this latter point, and may be convinced to abandon it.
> 
> I'm not convinced either.
> 
> > Backwards Compatibility
> > =======================
> >
> > This PEP neither removes current behavior of ``stat_result``, nor changes
> > the semantics of any current behavior.  Likewise, it adds functions and
> > aliases for functions to ``os.path``, but does not remove or change any
> > existing ones.
> >
> > Therefore, this PEP should not cause any backwards incompatibilities,
> > except in the rare and esoteric cases where code is dependent on the
> > *nonexistence* of the proposed new names.  It is not deemed important
> > remain compatible with code that mistakenly holds the Python Standard
> > Library to be closed for new additions.
> 
> You just want to make sure that your stat result is compatible with
> the current implementation (tuple-struct, supporting indexing).
> 
> > Open Issues
> > ===========
> >
> > Whether it is more desirable for the proposed added methods’ names to
> > follow PEP 8 [#PEP-8]_ (i.e.  ``is_file()`` etc.), or to mirror the
> > pre-existing names in ``os.path`` (i.e.  ``isfile()`` etc.) is still open
> > for debate.
> >
> > The existing attributes on ``stat_result`` follow the pattern ``st_*`` in
> > conformance to the relevant POSIX names for the fields of the C-level
> > ``stat`` structure.  The new names for the behaviours proposed here do not
> > contain such an ``st_`` prefix (nor could they, for that would suggest a
> > conformance with ``stat`` structure names which do not exist in POSIX).
> > But the resulting asymmetry of names is annoying.  Should aliases for the
> > existing ``st_*`` names be added that omit the ``st_`` prefix?
> 
> If we offer a higher level abstraction, then I think the 'st_' prefix
> should be dropped.
> 
> > As it stands, this PEP does not address the asymmetry between the existing
> > ``os.path.isfile()`` etc.  functions and the new proposed mechanism in the
> > case where the underlying file does not exist.  There is a way to handle
> > this, though: an optional flag could be added to ``os.*stat()`` that would
> > return a null object implementation of ``stat_result`` whenever the file
> > does not exist.  Then that null object could return ``False`` to
> > ``is_file()`` etc., That means that the following code would behave
> > identically, even when the file ``f`` does not exist::
> >
> >     if os.path.isfile(f) or os.path.isdir(f):
> >         # do something
> >
> >     st = os.stat(f, null_if_missing=True)
> >     if st.is_file() or st.is_dir():
> >         # do something
> >
> > Would this be a useful mechanism?
> 
> I don't like the idea of adding an optional attribute: stating a non
> existing file will return an exception, that's it.
> Also, I don't like the idea of a null object.
> 

-- 
Pieter Nagel