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

gh-115986 Improve pprint docs formatting by Privat33r-dev · Pull Request #117401 · python/cpython · GitHub

/ cpython Public

gh-115986 Improve pprint docs formatting - #117401

Merged
encukou merged 9 commits into
python:mainfrom
Privat33r-dev:add-params-table-115986
Jun 27, 2024
Merged

gh-115986 Improve pprint docs formatting#117401
encukou merged 9 commits into
python:mainfrom
Privat33r-dev:add-params-table-115986

Conversation

Privat33r-dev commented Mar 31, 2024
edited
Loading

Copy link
Copy Markdown
Contributor

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 force-pushed the add-params-table-115986 branch from 33e0842 to cc92c08 Compare March 31, 2024 12:16

Copy link
Copy Markdown
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 self-requested a review March 31, 2024 22:31

Privat33r-dev commented Apr 1, 2024
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
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

Copy link
Copy Markdown
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 changed the title gh-115986 Move pprinter parameters description to the table gh-115986 Improve pprint docs formatting Apr 30, 2024

Copy link
Copy Markdown
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 :)

Copy link
Copy Markdown
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

Copy link
Copy Markdown
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 commented May 2, 2024
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
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 commented May 2, 2024
edited
Loading

Copy link
Copy Markdown
Contributor Author

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:

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

Copy link
Copy Markdown
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 commented May 2, 2024
edited
Loading

Copy link
Copy Markdown
Contributor Author

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 commented May 2, 2024
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author


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.

Copy link
Copy Markdown
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.

Comment thread Doc/library/pprint.rst
*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
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

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
Copy Markdown
Contributor 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

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

Comment thread Doc/library/pprint.rst Outdated
Comment thread Doc/library/pprint.rst Outdated
Comment thread Doc/library/pprint.rst Outdated
Comment thread Doc/library/pprint.rst Outdated

hugovk commented Jun 26, 2024

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
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 merged commit 0890ad7 into python:main Jun 27, 2024

encukou commented Jun 27, 2024

Copy link
Copy Markdown
Member

Thank you!

encukou added the needs backport to 3.13 bugs and security fixes label Jun 27, 2024

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 <the.privat33r+gh@pm.me>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

bedevere-app Bot commented Jun 27, 2024

Copy link
Copy Markdown

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

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

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 <the.privat33r+gh@pm.me>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

bedevere-app Bot commented Jun 27, 2024

Copy link
Copy Markdown

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

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 <the.privat33r+gh@pm.me>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
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 <the.privat33r+gh@pm.me>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Petr Viktorin <encukou@gmail.com>
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 <1324225+hugovk@users.noreply.github.com>

Copy link
Copy Markdown
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 <1324225+hugovk@users.noreply.github.com>
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 <1324225+hugovk@users.noreply.github.com>
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
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


Back | FazBrowse Home | New Git URL