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.

Sign up for GitHub

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

Jump to bottom

gh-93096: Update and document pickle CLI #131097

Merged
merged 17 commits into from
Mar 14, 2025
Merged

gh-93096: Update and document pickle CLI #131097

merged 17 commits into from
Mar 14, 2025
+34 −13

Conversation

donBarbos
Copy link
Contributor

@donBarbos donBarbos commented Mar 11, 2025 •
edited
Loading

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news awaiting review labels Mar 11, 2025
@bedevere-app bedevere-app bot mentioned this pull request Mar 11, 2025
Open
27 tasks
@donBarbos
Copy link
Contributor Author

donBarbos commented Mar 11, 2025 •
edited
Loading

cc @vstinner as author of idea

in issue #130160 we're making backports for 3.12 and 3.13 versions. since this is a similar PR I suggest to stick to the same strategy

vstinner
vstinner approved these changes Mar 11, 2025
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

cc @picnixz

picnixz
picnixz reviewed Mar 12, 2025
View reviewed changes
@donBarbos @picnixz
Update Doc/library/pickle.rst
5909db4 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
hugovk
hugovk reviewed Mar 13, 2025
View reviewed changes
@donBarbos donBarbos requested review from hugovk and picnixz March 13, 2025 15:16
picnixz
picnixz reviewed Mar 13, 2025
View reviewed changes
picnixz
picnixz approved these changes Mar 13, 2025
View reviewed changes
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

Last nit and LGTM.

@picnixz picnixz changed the title gh-93096: Add CLI docs for pickle gh-93096: Update and document CLI pickle Mar 13, 2025
@picnixz picnixz changed the title gh-93096: Update and document CLI pickle gh-93096: Update and document pickle CLI Mar 13, 2025
@donBarbos @picnixz
Update Doc/library/pickle.rst
cefd8a4 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@picnixz
Copy link
Member

picnixz commented Mar 13, 2025

I'll wait for Hugo's review and then we can probably merge it (just for a last checkup)

vstinner
vstinner approved these changes Mar 14, 2025
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

@donBarbos @vstinner
Update Doc/library/pickle.rst
29d1360 
Co-authored-by: Victor Stinner <vstinner@python.org>
hugovk
hugovk approved these changes Mar 14, 2025
View reviewed changes
donBarbos and others added 2 commits March 14, 2025 10:15
@donBarbos @hugovk
Update Lib/pickle.py
3dd66f2 
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@donBarbos
back sort_dicts=True
ff22e97
hugovk
hugovk reviewed Mar 14, 2025
View reviewed changes
@hugovk
Copy link
Member

hugovk commented Mar 14, 2025

Do we need a NEWS file? This started out as docs only, but we're changing the code in pickle.py here. Although it's in code that was not documented before now, so maybe not needed?

@picnixz
Copy link
Member

picnixz commented Mar 14, 2025

We changed the code but it was already not documented. I don't think it's worth a What's New entry, but a small NEWS entry may be nice. We could make it under Library to mention that we now use pprint.pp instead of pprint.pprint to render pickle files.

@vstinner
Copy link
Member

Please don't replace pprint.pprint() with pprint.pp() in this PR but write a separated change for that.

@donBarbos
Copy link
Contributor Author

donBarbos commented Mar 14, 2025 •
edited
Loading

Ok, I reverted using pprint instead of pp

@hugovk
Copy link
Member

hugovk commented Mar 14, 2025

Thanks, remember to push :)

hugovk
hugovk reviewed Mar 14, 2025
View reviewed changes
@donBarbos @hugovk
Update Lib/pickle.py
8f1af6e 
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@vstinner vstinner merged commit f9d0531 into python:main Mar 14, 2025
39 checks passed
@vstinner
Copy link
Member

Merged, thank you.

plashchynski pushed a commit to plashchynski/cpython that referenced this pull request Mar 17, 2025
@donBarbos @picnixz
@vstinner @hugovk @plashchynski
pythongh-93096: Update and document pickle CLI (python#131097)
f430877 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@picnixz picnixz picnixz approved these changes

@vstinner vstinner vstinner approved these changes

@hugovk hugovk hugovk approved these changes

@JacobCoffee JacobCoffee JacobCoffee approved these changes

Assignees
No one assigned
Labels
docs Documentation in the Doc dir skip news
Projects
Docs PRs
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@donBarbos @picnixz @hugovk @vstinner @JacobCoffee