Skip to content

gh-115986 Improve pprint docs formatting #117401

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

Merged
merged 9 commits into from
Jun 27, 2024

Conversation

Privat33r-dev
Copy link
Contributor

@Privat33r-dev Privat33r-dev commented Mar 31, 2024

The change improves readability.
Suggested in the #116085 PR discussion.


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

The change improves readability.
Suggested in the GH#116085 PR discussion.
@Privat33r-dev Privat33r-dev force-pushed the add-params-table-115986 branch from 33e0842 to cc92c08 Compare March 31, 2024 12:16
@erlend-aasland
Copy link
Contributor

If we're putting the params list in the pprint.pprint function docs, we could just as well use the standardised Sphinx param list markup (:param:, :type:, etc.). We'll probably refer to pprint.pprint from the pprint.pp and the PrettyPrinter docs anyway, so using the standard markup seems like a better way to me. cc. @AlexWaygood @hugovk

@AlexWaygood AlexWaygood self-requested a review March 31, 2024 22:31
@Privat33r-dev
Copy link
Contributor Author

Privat33r-dev commented Apr 1, 2024

@erlend-aasland, I do completely agree that :param: will look better, especially considering the fact that we want to keep parameters' types as well and :param: look much more suitable for this purpose. I was hesitant to express my opinion earlier as everyone seemed to agree on table's solution, but now I see that it made more sense to speak my mind.

Moreover, I want to address the whole "pp vs pprint" thing. If we are writing docs from the developer's perspective, it would seem more logical to write the full description in one function's docs and just mention in another that it's an alias to keep everything DRY and readable (please, ignore formatting and minor details):

pprint.pp(object, stream=None, indent=1, width=80, depth=None, *, compact=False, sort_dicts=False, underscore_numbers=False)
Prints the formatted representation of object on stream, followed by a newline. If stream is None, [sys.stdout] is used. This may be used in the interactive interpreter instead of the [print()] function for inspecting values (you can even reassign print = pprint.pp for use within a scope).

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

** PrettyPrinter PARAMS HERE **

pprint.pprint(object, stream=None, indent=1, width=80, depth=None, *, compact=False, sort_dicts=True, underscore_numbers=False)
Alias to pprint.pp. Note that sort_dicts is True by default and you might want to use [pp()] instead where it is False by default.

It feels like it would make the most sense, because here we would give all the usage info on pprint.pp and only then mention "pprint.pprint" and other stuff.

@erlend-aasland
Copy link
Contributor

@Privat33r-dev, do you have a draft PR of your suggestion?

@Privat33r-dev
Copy link
Contributor Author

@Privat33r-dev, do you have a draft PR of your suggestion?

not yet, but I can make (or rather edit existing) one if my proposed change sounds good

@erlend-aasland
Copy link
Contributor

not yet, but I can make (or rather edit existing) one if my proposed change sounds good

I think a draft PR would be interesting, but only if you have time available for it; I find it easier to decide on doc layouts when they are rendered :)

@Privat33r-dev Privat33r-dev changed the title gh-115986 Move pprinter parameters description to the table gh-115986 Improve pprint docs formatting Apr 30, 2024
@Privat33r-dev
Copy link
Contributor Author

not yet, but I can make (or rather edit existing) one if my proposed change sounds good

I think a draft PR would be interesting, but only if you have time available for it; I find it easier to decide on doc layouts when they are rendered :)

done, you can check it out :)

@erlend-aasland
Copy link
Contributor

Thanks! I like this very much. We could use Sphinx placeholders to minimise the duplication. I'm not sure if it is worth it, though, because you'd have to create one placeholder for each parameter description.

Indentation of code blocks made them nested
"Version changed" is better placed after the code block
@Privat33r-dev
Copy link
Contributor Author

There are 2 differences between the params table:

  1. "object" presence as the first param
  2. (the default) placement for sort_dicts param

Even if we technically can make a placeholder for table (by "table" I mean params list), the first difference is resolved by putting :param: before the table, but the second difference could be resolved only by moving the table to pprint.pprint or by somehow replacing the content of the placeholder (which is kind of hacky).

We can keep it DAMP for now since there is low change of significant changes in the pprint.pprint and in case if there are some changes (which will unlikely involve parameters), we can add comment to rst that edit should be made twice.

@Privat33r-dev
Copy link
Contributor Author

Privat33r-dev commented May 2, 2024

My recent change
Move code out of parameter

Screenshots

Before

After

Move code out of "changed in version" and move the latter down
Screenshots

Before

After

@erlend-aasland
Copy link
Contributor

Moving the example code out of the parameter list sounds good. IMO we should never embed code examples in parameter lists, since it is (IMO) visually cluttering.

@Privat33r-dev
Copy link
Contributor Author

Privat33r-dev commented May 2, 2024

Moving the example code out of the parameter list sounds good. IMO we should never embed code examples in parameter lists, since it is (IMO) visually cluttering.

Moving example code in the param was unintentional consequence of adding params :)

I am not sure how to fix the tests though. Can you give an advise? This change will likely do the trick:
image

But I'd prefer to keep the indentation and avoid "usage example" and similar things to keep the document's consistency.

@erlend-aasland
Copy link
Contributor

I am not sure how to fix the tests though

You need to adjust the indentation so it matches the expected output; AFAIK, there is no other way.

@Privat33r-dev
Copy link
Contributor Author

Privat33r-dev commented May 2, 2024

I am not sure how to fix the tests though

You need to adjust the indentation so it matches the expected output; AFAIK, there is no other way.

putting additional newline between "usage example" and code fixed the indentation. I tried to use comment (.. example\n\n{code}), but it made example visually disappear (since I guess the code become child of comment). I guess we go with "usage example"s now. But, I guess, it makes the doc more explicit now :)

@Privat33r-dev
Copy link
Contributor Author

Privat33r-dev commented May 2, 2024

I figured out that the problem wasn't in the indentation of the code block but because my IDE decided to align code inside of the code blocks on <shift>+<tab>.

@Privat33r-dev
Copy link
Contributor Author

image
Well, after watching the Steven He's "Failure Management" course, I can confidently say that the problem is still in indentation and reducing amount of failures from 3 to 1 is a partial success.

@Privat33r-dev
Copy link
Contributor Author

Failure has been successfully managed. Ready for review :)

P.S. Why does doctest job take 16 minutes? Diving a bit deeper I noticed that doctest.py does not parallelize jobs, it might be the problem.


.. function:: pformat(object, indent=1, width=80, depth=None, *, \
compact=False, sort_dicts=True, underscore_numbers=False)

Return the formatted representation of *object* as a string. *indent*,
*width*, *depth*, *compact*, *sort_dicts* and *underscore_numbers* are
passed to the :class:`PrettyPrinter` constructor as formatting parameters
and their meanings are as described in its documentation below.
and their meanings are as described in the documentation above.
Copy link
Member

Choose a reason for hiding this comment

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

The documentation for PrettyPrinter is still below. Maybe this should be:

The arguments have the same meaning as for :func:`~pprint.pprint`.
Note that *sort_dicts* defaults to ``True``.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's why "its" was changed to "the", as arguments description is above (in the pp.pprint description).

@hugovk
Copy link
Member

hugovk commented Jun 26, 2024

I've merged main in, should fix the CI failure (check-warnings.py: error: unrecognized arguments: --fail-if-new-news-nit).

@Privat33r-dev
Copy link
Contributor Author

I've merged main in, should fix the CI failure (check-warnings.py: error: unrecognized arguments: --fail-if-new-news-nit).

Thanks. It worked :)

@encukou encukou merged commit 0890ad7 into python:main Jun 27, 2024
27 checks passed
@encukou
Copy link
Member

encukou commented Jun 27, 2024

Thank you!

@encukou encukou added the needs backport to 3.13 bugs and security fixes label Jun 27, 2024
@miss-islington-app

This comment was marked as outdated.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 27, 2024
* Move pprinter parameters description to the table

The change improves readability.
Suggested in the GHGH-116085 PR discussion.

* Make pprint doc with params markup

* Fix formatting
Indentation of code blocks made them nested
"Version changed" is better placed after the code block

* Fix formatting for tests

* fix code indentation for autotests

* Fix identation for autotests

* Remove duplication of the parameters' description

* Rearrange parameters description in a correct order

---------

(cherry picked from commit 0890ad7)

Co-authored-by: Kerim Kabirov <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jun 27, 2024

GH-121098 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jun 27, 2024
@encukou encukou added the needs backport to 3.12 only security fixes label Jun 27, 2024
@miss-islington-app

This comment was marked as outdated.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 27, 2024
* Move pprinter parameters description to the table

The change improves readability.
Suggested in the GHGH-116085 PR discussion.

* Make pprint doc with params markup

* Fix formatting
Indentation of code blocks made them nested
"Version changed" is better placed after the code block

* Fix formatting for tests

* fix code indentation for autotests

* Fix identation for autotests

* Remove duplication of the parameters' description

* Rearrange parameters description in a correct order

---------

(cherry picked from commit 0890ad7)

Co-authored-by: Kerim Kabirov <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jun 27, 2024

GH-121099 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Jun 27, 2024
encukou pushed a commit that referenced this pull request Jun 28, 2024
* Move pprinter parameter descriptions to a table

* Make pprint doc with params markup

* Remove duplication of the parameters' description

---------

(cherry picked from commit 0890ad7)

Co-authored-by: Kerim Kabirov <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
encukou added a commit that referenced this pull request Jun 28, 2024
gh-115986 Improve pprint docs formatting (GH-117401)

* Move pprinter parameters description to the table

The change improves readability.
Suggested in the GHGH-116085 PR discussion.

* Make pprint doc with params markup

* Fix formatting
Indentation of code blocks made them nested
"Version changed" is better placed after the code block

* Fix formatting for tests

* fix code indentation for autotests

* Fix identation for autotests

* Remove duplication of the parameters' description

* Rearrange parameters description in a correct order

---------

(cherry picked from commit 0890ad7)

Co-authored-by: Kerim Kabirov <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Petr Viktorin <[email protected]>
mrahtz pushed a commit to mrahtz/cpython that referenced this pull request Jun 30, 2024
* Move pprinter parameters description to the table

The change improves readability.
Suggested in the GH#116085 PR discussion.

* Make pprint doc with params markup

* Fix formatting
Indentation of code blocks made them nested
"Version changed" is better placed after the code block

* Fix formatting for tests

* fix code indentation for autotests

* Fix identation for autotests

* Remove duplication of the parameters' description

* Rearrange parameters description in a correct order

---------

Co-authored-by: Hugo van Kemenade <[email protected]>
@erlend-aasland
Copy link
Contributor

Next time, remember to edit the commit message upon merging, @encukou :)

Thanks for the contribution, Kerim!

noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
* Move pprinter parameters description to the table

The change improves readability.
Suggested in the GH#116085 PR discussion.

* Make pprint doc with params markup

* Fix formatting
Indentation of code blocks made them nested
"Version changed" is better placed after the code block

* Fix formatting for tests

* fix code indentation for autotests

* Fix identation for autotests

* Remove duplication of the parameters' description

* Rearrange parameters description in a correct order

---------

Co-authored-by: Hugo van Kemenade <[email protected]>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
* Move pprinter parameters description to the table

The change improves readability.
Suggested in the GH#116085 PR discussion.

* Make pprint doc with params markup

* Fix formatting
Indentation of code blocks made them nested
"Version changed" is better placed after the code block

* Fix formatting for tests

* fix code indentation for autotests

* Fix identation for autotests

* Remove duplication of the parameters' description

* Rearrange parameters description in a correct order

---------

Co-authored-by: Hugo van Kemenade <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants