-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
gh-83714: Use "stx_" prefix for all os.statx_result members #140432
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
Conversation
|
I'm not convinced that it's a good idea to have It's also strange that some statx members have a I don't think that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm 👍 on this as its evolved. There's too many weird edged cases and just separating entirely makes simpler to reason around.
I think if go forward with this all the stx_ attributes should be explicitly documented rather than refer to stat_result ones of different name. I'm often in a codebase wondering "what is this" and search in page to find; the text note "all the stat members but with a stx_ prefix" would break that
Doc/library/os.rst
Outdated
| :class:`!statx_result` is not a subclass of :class:`~stat_result` and cannot | ||
| be used as a tuple. | ||
|
|
||
| :class:`!statx_result` has the following additional attributes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should list all the attributes now; doc having people translate in their head rather than see "here's the name I used" seems like it will break "search in page" finding the attributes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I completed my PR to document all statx_members. It also clarifies which stat_result members are not part of statx_result:
- st_creator
- st_file_attributes
- st_flags
- st_fstype
- st_gen
- st_reparse_tag
- st_rsize
- st_type
Rename stx_birthtime to stx_btime, and rename stx_birthtime_ns to stx_btime_ns.
Remove now redundant MEMBER parameter.
|
I updated my PR to document all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 overall; curious @jbosboom thoughts. Appreciate the fixes/tweaks to Linux kernel version availability.
|
I changed my mind and just removed the comparison to stat_result:
I don't think that it's useful to compare these two structures, they are different and listing differences is not worth it. |
|
I think this needlessly complicates code that uses @vstinner You've made substantial changes to both the interface and implementation of |
Ok, I added myself to the credits. |
📚 Documentation preview 📚: https://cpython-previews--140432.org.readthedocs.build/