-
-
Notifications
You must be signed in to change notification settings - Fork 33k
gh-138952: Document platform.machine() output casing inconsistency across platforms #138962
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
Doc/library/platform.rst
Outdated
The output of :func:`platform.machine` is system-dependent and may differ in casing | ||
and naming conventions across platforms. For example, on Apple Silicon macOS it may | ||
return ``'arm64'`` (lowercase), while on Windows-on-ARM it may return ``'ARM64'`` (uppercase). | ||
If you need consistent results, normalize the output in your code. |
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.
Please wrap to 79 characters.
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.
Thanks! I’ve updated it.
Co-authored-by: AN Long <[email protected]>
See my comment on the issue suggesting any wording added here should not imply that the platform string can be parsed consistently for things like CPU architectures. |
What codebase generates those strings? Is it somehow the underlying native system? If Python docs recommend users to normalize the strings, shouldn't Python by itself Just Do It for the user? If not, why shouldn't it? I.e. this seems a bit like documenting/validating a bug as intended behavior, without an apparent reason of why that bug should be a feature in the first place. When ned mentions that platform string should not be parsed to find the current CPU architecture, then that is fine, and like ned pointed out in the ticket, that would be then good to be mentioned in documentation. It also then raises the obvious question of what should users utilize to detect the CPU architecture, if I.e. instead of documenting "This function should not be used to detect CPU architecture", a good documentation would state "This function should not be used to detect CPU architecture, because of X. To detect the CPU architecture, see Y instead." |
@ned-deily Thanks! for the feedback.
Please let me know if I’ve misunderstood your suggestion. |
Doc/library/platform.rst
Outdated
The output of :func:`platform.machine` is system-dependent and may differ | ||
in casing and naming conventions across platforms. For example, on Apple | ||
Silicon based macOS it may return ``'arm64'`` (lowercase), while on Windows-on-ARM | ||
it may return ``'ARM64'`` (uppercase). If you need consistent results, | ||
normalize the output in your code. |
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.
IMO, I think this whole section is a little excessive, I think just this would be fine:
.. function:: machine()
Returns the machine type, e.g. ``'AMD64'``. An empty string is returned if the
value cannot be determined.
The output is platform-dependent and may differ in casing and naming
conventions.
If we do want to stay with the note, please remove the link to the function, it is unnecessary.
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.
Should I update the changes as per your suggestions and remove the note section?
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 agree with @StanFromIreland's suggestion here. @Aniketsy, it would be nice if you could update the PR accordingly.
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.
@ned-deily I’ve updated the PR as suggested.
Thanks @Aniketsy for the PR, and @ned-deily for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Thanks @Aniketsy for the PR, and @ned-deily for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
…ncy across platforms (pythonGH-138962) (cherry picked from commit dd15a2e) Co-authored-by: Aniket <[email protected]> Co-authored-by: AN Long <[email protected]>
…ncy across platforms (pythonGH-138962) (cherry picked from commit dd15a2e) Co-authored-by: Aniket <[email protected]> Co-authored-by: AN Long <[email protected]>
GH-139045 is a backport of this pull request to the 3.14 branch. |
GH-139046 is a backport of this pull request to the 3.13 branch. |
…ency across platforms (GH-138962) (#139046) (cherry picked from commit dd15a2e) Co-authored-by: Aniket <[email protected]> Co-authored-by: AN Long <[email protected]>
…ency across platforms (GH-138962) (#139045) gh-138952: Document platform.machine() output casing inconsistency across platforms (GH-138962) (cherry picked from commit dd15a2e) Co-authored-by: Aniket <[email protected]> Co-authored-by: AN Long <[email protected]>
This PR updates the documentation for
platform.machine()
inplatform.rst
to clarify that the output is system-dependent and may differ in casing and naming conventions across platforms.For example, on Apple Silicon macOS it may return 'arm64' (lowercase), while on Windows-on-ARM it may return 'ARM64' (uppercase).
Please let me know if my approach or fix needs any improvements . I’m open to feedback and happy to make changes based on suggestions.
Thankyou !
📚 Documentation preview 📚: https://cpython-previews--138962.org.readthedocs.build/