GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-115986: Use param list to mark up pprint.PrettyPrinter constructor by erlend-aasland · Pull Request #116085 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Feb 28, 2024 •
edited by hugovk
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • Issue: Improve documentation for pprint module #115986

📚 Documentation preview 📚: https://cpython-previews--116085.org.readthedocs.build/en/116085/library/pprint.html

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@erlend-aasland
pythongh-115986: Use param list to mark up pprint.PrettyPrinter const…
451067b
…ructor
@erlend-aasland erlend-aasland added skip news needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Feb 28, 2024
@erlend-aasland
erlend-aasland requested review from AlexWaygood and hugovk February 28, 2024 23:15
@bedevere-app bedevere-app Bot added the awaiting core review label Feb 28, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Feb 28, 2024
Improve documentation for pprint module #115986
Closed
@bedevere-app bedevere-app Bot added the docs Documentation in the Doc dir label Feb 28, 2024
@erlend-aasland

erlend-aasland commented Feb 28, 2024

Copy link
Copy Markdown
Contributor Author

cc. @Privat33r-dev

Privat33r-dev reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Privat33r-dev

Privat33r-dev commented Feb 29, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Side note:

The configuration parameters stream, indent, width, depth, compact, sort_dicts and underscore_numbers are passed to the PrettyPrinter constructor and their meanings are as described in its documentation above.

Just noticed 2 documentation above mentions, which is inaccurate after we moved class to the bottom.

AlexWaygood and erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Privat33r-dev
Privat33r-dev reviewed Feb 29, 2024
View reviewed changes
Comment thread Doc/library/pprint.rst

:param bool sort_dicts:
If ``True`` (the default), dictionaries will be formatted with
their keys sorted, otherwise they will display in insertion order.

@Privat33r-dev Privat33r-dev Feb 29, 2024

Copy link
Copy Markdown
Contributor

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.

Maybe a passive tense would be more accurate choice? will be displayed. Though I am not sure.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@erlend-aasland

erlend-aasland commented Feb 29, 2024

Copy link
Copy Markdown
Contributor Author

Just noticed 2 documentation above mentions, which is inaccurate after we moved class to the bottom.

Good catch. Can you please open a PR to fix that?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood
AlexWaygood reviewed Feb 29, 2024
View reviewed changes

@AlexWaygood AlexWaygood left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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 think the fundamental problem with this module's docs is that they've been written by somebody thinking about it from the perspective of how the module is implemented, rather than from the perspective of how users generally use it. From the perspective of the implementation, it makes sense to document the meaning of all these parameters as part of the PrettyPrinter class, since all the module-level functions are just abstractions over that class. But from the perspective of the user, I'm not sure that really makes sense — users of this module rarely need to use the low-level class.

So maybe the meaning of these parameters should be documented in a separate table somewhere in these docs, rather than as part of the documentation of this class — we could just say the parameters have the same meaning for all functions in the module

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland, hugovk, and Privat33r-dev reacted with thumbs up emoji erlend-aasland reacted with rocket emoji
All reactions
  • 👍 3 reactions
  • 🚀 1 reaction
@Privat33r-dev

Privat33r-dev commented Feb 29, 2024

Copy link
Copy Markdown
Contributor

So maybe the meaning of these parameters should be documented in a separate table somewhere in these docs, rather than as part of the documentation of this class — we could just say the parameters have the same meaning for all functions in the module

Is there an example of this style? I would definitely agree that we should make params for pprint functions more visible.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Privat33r-dev

Privat33r-dev commented Feb 29, 2024

Copy link
Copy Markdown
Contributor

Good catch. Can you please open a PR to fix that?

#116104

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood

AlexWaygood commented Feb 29, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Is there an example of this style?

I'm imagining a table that looks a bit like https://docs.python.org/3/library/typing.html#id7 (source code at

cpython/Doc/library/typing.rst

Line 2736 in 86e5e06

.. list-table:: **Recognised parameters for field specifiers**
)

erlend-aasland and Privat33r-dev reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented Feb 29, 2024

Copy link
Copy Markdown
Contributor Author

I agree, @AlexWaygood. We ran into similar problems in the ftplib docs. Perhaps we should consider this approach over there as well.

I'll close this PR for now.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland erlend-aasland closed this Feb 29, 2024
@erlend-aasland
erlend-aasland deleted the docs/pprint/paramlist-for-prettyprinter-init branch February 29, 2024 10:25
@Privat33r-dev

Privat33r-dev commented Mar 10, 2024

Copy link
Copy Markdown
Contributor

Is there an example of this style?

I'm imagining a table that looks a bit like https://docs.python.org/3/library/typing.html#id7 (source code at

cpython/Doc/library/typing.rst

Line 2736 in 86e5e06

.. list-table:: **Recognised parameters for field specifiers**

)

I wish there was a more modern design for the table though. I am not sure where I can post proposal for it, but it would be nice if instead of this:
image

It looked at least like this:
image

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented Mar 10, 2024

Copy link
Copy Markdown
Contributor Author

@Privat33r-dev, I'd try and ask in the Documentation category on Discourse, or on our Python Docs Discord server.

Privat33r-dev and AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Privat33r-dev Privat33r-dev mentioned this pull request Mar 11, 2024
GH-115986 Docs: promote pprint.pp usage as a default #116614
Merged
@Privat33r-dev Privat33r-dev mentioned this pull request Mar 31, 2024
gh-115986 Improve pprint docs formatting #117401
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood left review comments
@hugovk hugovk Awaiting requested review from hugovk
+1 more reviewer
@Privat33r-dev Privat33r-dev Privat33r-dev left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting core review docs Documentation in the Doc dir needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@erlend-aasland @Privat33r-dev @AlexWaygood
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.