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

bpo-30670: Add pp function to the pprint module #11769

Merged
merged 10 commits into from
Mar 22, 2019

Conversation

remilapeyre
Copy link
Contributor

@remilapeyre remilapeyre commented Feb 6, 2019

Add sort_dicts to pprint.PrettyPrinter, pprint.pformat and pprint.pprint

https://bugs.python.org/issue30670

Add sort_dicts to pprint.PrettyPrinter, pprint.pformat and pprint.pprint
Copy link
Contributor

@eamanu eamanu left a comment

Choose a reason for hiding this comment

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

I test the behavior (ubuntu 18.04) and this PR accomplish with the bpo. IMO, after solve the Travis CI error this should be ready for merge. Before be review by a core dev :-)

Added the *sort_dicts* parameter.


.. function:: pp(object, *args, sort_dicts=False, **kwargs)

Choose a reason for hiding this comment

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

Would it be worth explicitely saying that this is an alias of pprint() with sort_dict=False ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is already a mention of *args and **kwargs being passed to pprint and sort_dicts=False is in the signature of pp so I don't think it is necessary.

I would like to also like to avoid giving to much details about pp implementation because we may change some things if another data structure of Python changes like dicts did.

Choose a reason for hiding this comment

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

(if we were to do change what this function does, we would also change the doc, but I'm good with that.)

Choose a reason for hiding this comment

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

Would it be worth explicitely saying that this is an alias of pprint() with sort_dict=False ?

At first glance I was puzzled why both pp() and pprint() existed, and it took a moment to spot the different default sort_dict value - and I ended up having to confirm via source code inspection that there were no other differences between the two.

As such, the above mentioned docs would have been useful for me personally :-)

@remilapeyre
Copy link
Contributor Author

Thanks @vadmium!

* Add "pp" to __all__
* Drop mention of alphabetical sorting. It is just plain sorting
  (i.e. dict with integer keys get sorted as well).
* Fix spelling of "parameter"
@rhettinger rhettinger merged commit 96831c7 into python:master Mar 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants