Skip to content

gh-99146 struct module documentation should have more predictable examples/warnings #99141

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

Merged
merged 20 commits into from
Nov 22, 2022

Conversation

smontanaro
Copy link
Contributor

@smontanaro smontanaro commented Nov 5, 2022

This PR contains three changes:

  1. Modifies the introductory paragraph to be a bit more direct.
  2. Tweaked the first example to work on most common processors. As is, it fails on both Intel x86 and Apple M1 processors.
  3. Similarly, tweaked the third example to hopefully work in more cases.

@bedevere-bot bedevere-bot added awaiting review docs Documentation in the Doc dir skip news labels Nov 5, 2022
@smontanaro smontanaro marked this pull request as draft November 5, 2022 21:32
@smontanaro smontanaro changed the title nail down a couple examples to have more predictable output gh-99146 nail down a couple examples to have more predictable output Nov 5, 2022
Copy link
Member

@mdickinson mdickinson left a comment

Choose a reason for hiding this comment

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

Changes LGTM

@merwok
Copy link
Member

merwok commented Nov 6, 2022

Could the PR title (which will become the commit message) be less colloquial and mention that this is about struct doc?

@smontanaro smontanaro changed the title gh-99146 nail down a couple examples to have more predictable output gh-99146 struct module documentation should have more predictable examples/warnings Nov 6, 2022
@smontanaro
Copy link
Contributor Author

@merwok Sorry, updated.

@gpshead gpshead added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Nov 7, 2022
@smontanaro
Copy link
Contributor Author

I added an Applications section which discusses the different needs of native and machine-independent formats. I also made sure all format strings contain a prefix character.

@smontanaro smontanaro marked this pull request as ready for review November 12, 2022 15:55
@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

mostly good, a few suggestions.

Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

Thanks for working through all this. The docs are better as a result. :)

@smontanaro
Copy link
Contributor Author

Trust me, @gpshead, I got more out of the process than Python did. Thanks for the help/patience/feeback.

Onto my next random module...

@smontanaro
Copy link
Contributor Author

@mdickinson or @gpshead Can you merge with you have a moment? Thx...

@mdickinson
Copy link
Member

Looks very good to me. Since @gpshead has approved, I'll merge as soon as the CI completes.

@mdickinson mdickinson merged commit 22d91c1 into python:main Nov 22, 2022
@miss-islington
Copy link
Contributor

Thanks @smontanaro for the PR, and @mdickinson for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 22, 2022
…le examples/warnings (pythonGH-99141)

* nail down a couple examples to have more predictable output

* update a number of things, but this is really just a stash...

* added an applications section to describe typical uses for native and machine-independent formats

* make sure all format strings use a format prefix character

* responding to comments from @gpshead. Not likely finished yet.

* This got more involved than I expected...

* respond to several PR comments
* a lot of wordsmithing
* try and be more consistent in use of ``x`` vs ``'x'``
* expand examples a bit
* update the "see also" to be more up-to-date
* original examples relied on import * so present all examples as if

* reformat based on @gpshead comment (missed before)

* responding to comments

* missed this

* one more suggested edit

* wordsmithing
(cherry picked from commit 22d91c1)

Co-authored-by: Skip Montanaro <[email protected]>
@bedevere-bot
Copy link

GH-99702 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Nov 22, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 22, 2022
…le examples/warnings (pythonGH-99141)

* nail down a couple examples to have more predictable output

* update a number of things, but this is really just a stash...

* added an applications section to describe typical uses for native and machine-independent formats

* make sure all format strings use a format prefix character

* responding to comments from @gpshead. Not likely finished yet.

* This got more involved than I expected...

* respond to several PR comments
* a lot of wordsmithing
* try and be more consistent in use of ``x`` vs ``'x'``
* expand examples a bit
* update the "see also" to be more up-to-date
* original examples relied on import * so present all examples as if

* reformat based on @gpshead comment (missed before)

* responding to comments

* missed this

* one more suggested edit

* wordsmithing
(cherry picked from commit 22d91c1)

Co-authored-by: Skip Montanaro <[email protected]>
@bedevere-bot
Copy link

GH-99703 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Nov 22, 2022
mdickinson pushed a commit that referenced this pull request Nov 22, 2022
…ble examples/warnings (GH-99141) (GH-99702)

gh-99146 struct module documentation should have more predictable examples/warnings (GH-99141)

* nail down a couple examples to have more predictable output

* update a number of things, but this is really just a stash...

* added an applications section to describe typical uses for native and machine-independent formats

* make sure all format strings use a format prefix character

* responding to comments from @gpshead. Not likely finished yet.

* This got more involved than I expected...

* respond to several PR comments
* a lot of wordsmithing
* try and be more consistent in use of ``x`` vs ``'x'``
* expand examples a bit
* update the "see also" to be more up-to-date
* original examples relied on import * so present all examples as if

* reformat based on @gpshead comment (missed before)

* responding to comments

* missed this

* one more suggested edit

* wordsmithing
(cherry picked from commit 22d91c1)

Co-authored-by: Skip Montanaro <[email protected]>

Co-authored-by: Skip Montanaro <[email protected]>
mdickinson pushed a commit that referenced this pull request Nov 22, 2022
…ble examples/warnings (GH-99141) (GH-99703)

gh-99146 struct module documentation should have more predictable examples/warnings (GH-99141)

* nail down a couple examples to have more predictable output

* update a number of things, but this is really just a stash...

* added an applications section to describe typical uses for native and machine-independent formats

* make sure all format strings use a format prefix character

* responding to comments from @gpshead. Not likely finished yet.

* This got more involved than I expected...

* respond to several PR comments
* a lot of wordsmithing
* try and be more consistent in use of ``x`` vs ``'x'``
* expand examples a bit
* update the "see also" to be more up-to-date
* original examples relied on import * so present all examples as if

* reformat based on @gpshead comment (missed before)

* responding to comments

* missed this

* one more suggested edit

* wordsmithing
(cherry picked from commit 22d91c1)

Co-authored-by: Skip Montanaro <[email protected]>

Co-authored-by: Skip Montanaro <[email protected]>
@smontanaro smontanaro deleted the struct-doc branch November 22, 2022 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants