Skip to content

gh-93096: Update and document pickle CLI#131097

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
Merged
vstinner merged 17 commits into from
Mar 14, 2025
Merged

gh-93096: Update and document pickle CLI #131097

vstinner merged 17 commits into from
Mar 14, 2025

Conversation

@donbarbos
Copy link
Contributor

@donbarbosdonbarbos commented Mar 11, 2025•
edited
Loading

@bedevere-appbedevere-appbot added docs Documentation in the Doc dir skip news awaiting review labels Mar 11, 2025
@bedevere-appbedevere-appbot mentioned this pull request Mar 11, 2025
Open
27 tasks
@donbarbos
Copy link
ContributorAuthor

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

@vstinnervstinner 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
@donbarbosdonbarbos requested review from hugovk and picnixzMarch 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

@picnixzpicnixz 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.

@picnixzpicnixz changed the title gh-93096: Add CLI docs for picklegh-93096: Update and document CLI pickleMar 13, 2025
@picnixzpicnixz changed the title gh-93096: Update and document CLI picklegh-93096: Update and document pickle CLIMar 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

@vstinnervstinner 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
donbarbosand 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

vstinner commented Mar 14, 2025

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

@donbarbos
Copy link
ContributorAuthor

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>
@vstinnervstinner merged commit f9d0531 into python:mainMar 14, 2025
39 checks passed
@github-project-automationgithub-project-automationbot moved this from Todo to Done in Docs PRsMar 14, 2025
@vstinner
Copy link
Member

vstinner commented Mar 14, 2025

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 documentpickleCLI (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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@picnixzpicnixzpicnixz approved these changes

@vstinnervstinnervstinner approved these changes

@hugovkhugovkhugovk approved these changes

@JacobCoffeeJacobCoffeeJacobCoffee approved these changes

Assignees

No one assigned

Labels

docsDocumentation in the Doc dirskip 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