Harmonize os stubs for 2 and 3 #1324
Conversation
And fix some TODOs. This will help me check these stubs with my stubcheck tool, and is better for consistency anyway.
In preparation for merging. I avoided making other changes to the bugs but added some TODOs for missing stuff that I noticed.
|
I decided to broaden this PR a bit to also merge the stubs for 2 and 3. (This module is really big so there are constant changes, but there's really nothing essentially different between Python 2 and 3.) The commits in the PR should be self-contained (first one is moving |
|
Actually I am going to hold off on merging the two files because I'll probably have to merge os.path at the same time. Therefore, this PR will just be about making the two os/init.pyi files identical. |
72f5059 to
803b32a
Compare
|
I'm hanging on to this review for a bit, it looks more complex than most. |
| # accessible as a tuple of at least 10 integers giving the most important | ||
| # (and portable) members of the stat structure, in the order st_mode, | ||
| # st_ino, st_dev, st_nlink, st_uid, st_gid, st_size, st_atime, st_mtime, | ||
| # st_ctime. More items may be added at the end by some implementations. |
There was a problem hiding this comment.
Really? That would make classic tuple unpacking impossible. Where did you read that?
| st_creator: int | ||
| st_type: int | ||
|
|
||
| class statvfs_result: # Unix only |
There was a problem hiding this comment.
This doesn't work -- the real os.statvfs_result class is also sequence of 10 integers. I found working code that breaks because of this. (And yes, this means that the PY3 stubs are also broken. Too bad. It still works.) Maybe for this a NamedTuple could work, even if it can't for os.stat_result. (I wonder if for the latter we could start with a namedtuple and then just subclass it and add a bunch more fields as properties or maybe just class attributes?)
|
(And I'm not done reviewing, I'm just not sure I can stomach the review.) |
|
Thanks! I can try splitting this up if it does end up making things easier. |
|
Oh, I found the docs about stat() returning an N-tuple for N >= 10 here: https://docs.python.org/3/library/os.html?highlight=os.stat#os.stat_result.st_file_attributes (in the next paragraph). But I think that's wrong, the source hard-codes the size as 10: https://github.com/python/cpython/blob/master/Modules/posixmodule.c#L1746 and https://github.com/python/cpython/blob/master/Modules/posixmodule.c#L1845 (and corresponding PY2 code is the same). So apart from platforms that are neither posix nor windows, it's always 10, and that's good enough for me (platforms that don't have this == 10 will be unusable in practice). |
|
I can try splitting this up if it does end up making things easier.
Yes please.
|
Two major changes:
E_SPAM: intinstead ofE = 0).Also added some TODOs for missing arguments I noticed and a few other minor changes.