Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Reverted] Make os.stat_result and friends NamedTuples #1103

Merged
merged 1 commit into from
Mar 29, 2017

Conversation

euresti
Copy link
Contributor

@euresti euresti commented Mar 26, 2017

The elements can be accessed as indexes or through attributes, so I chose to make them NamedTuples.

('f_namemax', int)])

class stat_result(NamedTuple('stat_result', [
('st_mode', int), # protection bits,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need all of those comments. Typeshed is for types, not documentation.

# st_ctime. More items may be added at the end by some implementations.

if sys.version_info >= (3, 3):
st_atime_ns = 0 # time of most recent access, in nanoseconds
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to write st_atime_ns: int (or st_atime_ns = ... # type: int, but I think we've established that all users of typeshed obey PEP 526 now).

@@ -325,8 +321,11 @@ if sys.version_info >= (3, 5):
def scandir(path: str = ...) -> Iterator[DirEntry[str]]: ...
@overload
def scandir(path: bytes) -> Iterator[DirEntry[bytes]]: ...
@overload
def stat_float_times(newvalue: bool = ...) -> None: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should have a default value; that will make it overlap with the other overload.

@JelleZijlstra JelleZijlstra merged commit 6c3e175 into python:master Mar 29, 2017
@JelleZijlstra
Copy link
Member

Thanks! Merged.

@gvanrossum
Copy link
Member

Hmm... This caused two types of breakage with our internal code. I think it's better to roll back. @JelleZijlstra, @euresti What do you guys think? Here are the errors (same as I reported for #1111):

This one's caused by the (newly introduced) difference between os.stat_result and posix.stat_result:

Argument 1 to "check_perm" has incompatible type "stat_result"; expected "posix.stat_result"

This one's triggered by os.stat(path)[stats.ST_MODE]:

Tuple index must be an integer literal

Who agrees with a rollback?

@JelleZijlstra
Copy link
Member

We can fix the first one pretty easily by making stdlib/2/posix.pyi re-export os.stat_result (the one in stdlib/3 already does that).

I don't think the second one can be fixed in the stubs, but maybe mypy is being too strict there (it could just allow that code and make it return Any).

I'm OK with reverting for now, but it would be good to get some form of this PR back in eventually so os.stat doesn't just return Any.

@gvanrossum
Copy link
Member

I filed python/mypy#3078 for the latter. I'll revert if you don't beat me to it.

gvanrossum added a commit that referenced this pull request Mar 29, 2017
@gvanrossum gvanrossum changed the title Make os.stat_result and friends NamedTuples [Reverted] Make os.stat_result and friends NamedTuples Mar 29, 2017
@euresti
Copy link
Contributor Author

euresti commented Mar 29, 2017 via email

@euresti euresti deleted the os_stat branch December 14, 2017 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants