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.

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

gh-115986: Use param list to mark up pprint.PrettyPrinter constructor #116085

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Feb 28, 2024

@erlend-aasland
Copy link
Contributor Author

cc. @Privat33r-dev

@Privat33r-dev
Copy link
Contributor

Privat33r-dev commented Feb 29, 2024

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.


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

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.

@erlend-aasland
Copy link
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?

Copy link
Member

@AlexWaygood AlexWaygood left a 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

@Privat33r-dev
Copy link
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.

@Privat33r-dev
Copy link
Contributor

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

#116104

@AlexWaygood
Copy link
Member

AlexWaygood commented Feb 29, 2024

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

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

@erlend-aasland
Copy link
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.

@erlend-aasland erlend-aasland deleted the docs/pprint/paramlist-for-prettyprinter-init branch February 29, 2024 10:25
@Privat33r-dev
Copy link
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

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

@erlend-aasland
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review docs Documentation in the Doc dir needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants