KEMBAR78
gh-83714: Use "stx_" prefix for all os.statx_result members by vstinner · Pull Request #140432 · python/cpython · GitHub
Skip to content

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Oct 21, 2025

@vstinner
Copy link
Member Author

I'm not convinced that it's a good idea to have os.statx_result which can be used instead of os.stat_result. It's not exactly the same, os.statx_result members can be None.

It's also strange that some statx members have a st_ prefix, whereas others have a stx_ prefix. I read the statx manual page and expect to have only stx_ prefix: same members names.

I don't think that stx_ prefix would prevent using statx() in pathlib in the future.

cc @jbosboom @cmaloney

Copy link
Contributor

@cmaloney cmaloney left a 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

: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:
Copy link
Contributor

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.

Copy link
Member Author

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.
@vstinner
Copy link
Member Author

I updated my PR to document all statx_result members, and I renamed stx_birthtime to stx_btime.

Copy link
Contributor

@cmaloney cmaloney left a 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.

@vstinner
Copy link
Member Author

I changed my mind and just removed the comparison to stat_result:

statx_result has all the attributes that stat_result has on Linux, but using stx_ prefix instead of st_. statx_result is not a subclass of stat_result and cannot be used as a tuple.

I don't think that it's useful to compare these two structures, they are different and listing differences is not worth it.

@jbosboom
Copy link
Contributor

I think this needlessly complicates code that uses os.statx on Python >= 3.15 and falls back to os.stat on earlier versions, but I guess there won't be any user code doing that if we get the pathlib interface in 3.15 as well, so +0 from me.

@vstinner You've made substantial changes to both the interface and implementation of os.statx. I think you ought to add your name in the blurb for the issue.

@vstinner vstinner requested a review from AA-Turner as a code owner October 22, 2025 08:33
@vstinner
Copy link
Member Author

@vstinner You've made substantial changes to both the interface and implementation of os.statx. I think you ought to add your name in the blurb for the issue.

Ok, I added myself to the credits.

@vstinner vstinner merged commit 7339cf7 into python:main Oct 22, 2025
45 checks passed
@vstinner vstinner deleted the statx_stx branch October 22, 2025 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants