FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-149026: Add colour to `pickletools` CLI output by hugovk · Pull Request #149027 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-149026: Add colour to pickletools CLI output - #149027

Merged
hugovk merged 7 commits into
python:mainfrom
hugovk:3.15-pickletools-colour
Apr 29, 2026
ConversationCommits (7)ChecksFiles changed
Merged

gh-149026: Add colour to pickletools CLI output#149027
hugovk merged 7 commits into
python:mainfrom
hugovk:3.15-pickletools-colour

Conversation

hugovk commented Apr 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Create a pickle file, for example:

import pickle

data = {
    "a": [1, 2.0, 3 + 4j],
    "b": ("character string", b"byte string"),
    "c": {None, True, False},
}

with open("data.pickle", "wb") as f:
    pickle.dump(data, f, protocol=5)

Then ./python.exe -m pickletools data.pickle:

📚 Documentation preview 📚: https://cpython-previews--149027.org.readthedocs.build/

hugovk requested a review from AA-Turner as a code owner April 26, 2026 20:06
hugovk added the stdlib Standard Library Python modules in the Lib/ directory label Apr 26, 2026
bedevere-app Bot mentioned this pull request Apr 26, 2026
Closed

hugovk commented Apr 26, 2026

Copy link
Copy Markdown
Member Author

Aside: the alignment is the same in main without colour, and the code says:

            # make a mild effort to align arguments
            ...
            # make a mild effort to align annotations
            ...

If someone's up for making a bit less mild effort in another PR, go for it!

picnixz commented Apr 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Can we have the instruction names in white (no color)? I find it less readable with colors as all instructions have the same color.

hugovk commented Apr 27, 2026

Copy link
Copy Markdown
Member Author

Like this?

Or I think bold cyan is better?

picnixz commented Apr 27, 2026

Copy link
Copy Markdown
Member

IMO, too many colors hurt more than they help. White is not that good either actually... I do like the colors for literals and the first column but I feel that colors for instruction names are always too much because it's just adding colors to words that will anyway be classified identically.

Alternatively, how about some "dead" words such as MARK/NONE etc and add categorize the opcodes themselves into colorized categories. This could help a bit more visually (e.g., I want to locate all MARK, whether I use colors or not now I won't be able to find it easily without a CTRL+F).

hugovk commented Apr 27, 2026

Copy link
Copy Markdown
Member Author

Something like this?

picnixz commented Apr 27, 2026

Copy link
Copy Markdown
Member

Much better IMO! I would suggest not having colors for opcodes that have a colored literal (like SHORT_BINUNICODE). WDYT?

hugovk commented Apr 27, 2026

Copy link
Copy Markdown
Member Author

Or we could stripe it to match the value? But give the mild attempt at alignment, maybe they're a bit too close.

picnixz commented Apr 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Or we could stripe it to match the value? But give the mild attempt at alignment, maybe they're a bit too close.

It would require a bit more effort to parse the literal in this case. I think the no-color option is better IMO. FTR, the MARK can be in gray as the PROTO because each MARK is followed by an indent I thnk.

hugovk commented Apr 27, 2026

Copy link
Copy Markdown
Member Author

MARK in grey:

picnixz commented Apr 27, 2026

Copy link
Copy Markdown
Member

I like those colors, at least they are more readable to me (and I have very poor eyesight). What about light theme though?

hugovk commented Apr 27, 2026

Copy link
Copy Markdown
Member Author

picnixz reviewed Apr 27, 2026
View reviewed changes

picnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I find the colors much readable now! thanks for the work

Comment thread Lib/pickletools.py Outdated
Comment thread Lib/pickletools.py
Comment thread Lib/pickletools.py
hugovk and others added 3 commits April 27, 2026 14:47
Improve readability
9db621e 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Simplify
2d232cd
Fix GH web editor's weird line endings
b720bd0

hugovk commented Apr 29, 2026

Copy link
Copy Markdown
Member Author

@picnixz Any further comments or ready to merge?

picnixz reviewed Apr 29, 2026
View reviewed changes
Comment thread Lib/pickletools.py Outdated
# Group opcode names into categories for colourised CLI output.
_opcode_categories = frozendict(
op_call=frozenset({
"BUILD", "EXT1", "EXT2", "EXT4", "GLOBAL", "INST", "NEWOBJ",

picnixz Apr 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Quick question but can be done in a follow-up:

  • Are those valid for any pickle protocol version?
  • Check that the OP codes are valid using opcodes. It contains all opcodes + their protocol versions. You can use name2i but you need to remove the del name2i above in the file. Or you can build this list closer to name2i (L2200)

picnixz Apr 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Alternative: add a "category" to the opcode objects. Thankfully, those objects are not tuples so we can't break things.

hugovk Apr 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, it covers all protocol versions. Adding a "category" is an interesting idea, but not sure this should be part of the public API. I've gone for the first suggestion.

picnixz Apr 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Well, you can always make it a private attribute :) but it's not really important I think. We can revisit this later. It's only a burden when we create the list. But adding new opcodes is usually not common so... it's not really important.

read-the-docs-community Bot commented Apr 29, 2026

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #32468999 | 📁 Comparing 5338435 against main (1e7dfbc)

  🔍 Preview build  

5 files changed · ± 5 modified

± Modified

hugovk merged commit 8851a06 into python:main Apr 29, 2026
60 checks passed
hugovk deleted the 3.15-pickletools-colour branch April 29, 2026 15:33

hugovk commented Apr 29, 2026

Copy link
Copy Markdown
Member Author

Thanks for the review!

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

picnixz picnixz approved these changes
AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner

Assignees

No one assigned

Labels

stdlib Standard Library Python modules in the Lib/ directory

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL