Vendor in and absorb the pprint module from upstream#11626
Vendor in and absorb the pprint module from upstream#11626bluetech merged 7 commits intopytest-dev:mainfrom
Conversation
BenjaminSchubert
force-pushed
the
bschubert/vendored-pprint
branch
from
7b1657d to
92ecd40
Compare
bluetech
left a comment
bluetech
left a comment
There was a problem hiding this comment.
Thanks! LGTM, and was very easy to review. I'll merge this in a couple of days to give others a chance to review.
And finally, I believe there are some cleanups I believe we can do later on on the pprint module (adding types, cleaning up data structures). Is that something you would like or would you rather keep it closer to upstream? I can make a PR once both this one and #11571 are done if you wish
I think we can diverge from upstream. So 👍 on further cleanups/typings from me.
|
@BenjaminSchubert thanks for applying the suggestion! Would you mind squashing the last 3 commits? I would like us to merge this PR including your individual commits, as they contain valuable context -- I mention this because we often squash PRs in a single commit, but I think this particular commit history worth keeping. |
We already have the AlwaysDispatchingPrettyPrinter override of the default pretty printer. In order to make more in depth changes, we need to copy the upstream version in, as it doesn't lend itself well to being extended. This does a verbatime copy, adding provenance information at the top.
There are parts of the original pprint module that we won't need, let's limit the surface and remove the unnecessary code
There is more type information that could be added. We can add those later to make it easier, this is jsut the minimum to allow linting to pass
We don't need to keep the separation anymore, and this will make it easier to extend
BenjaminSchubert
force-pushed
the
bschubert/vendored-pprint
branch
from
c78a914 to
53d7b5e
Compare
|
@nicoddemus Done, I also rebased on the main branch at the same time |
3 participants
Overview
As part of #11571, where lots of changes are added to how diffs are pretty printed, we need to vendor in the pprint module from cpython.
This is a separate PR to do just that for easier reviews.
Notes for maintainers
I was not sure if we should keep the original 'Author' from CPython in the comment at the top of the module or if we should put another type of heading.
I have also added some tests (adapted from #11571) to ensure this functionality was working for when we get there.
@bluetech I ended up splitting it in more commits than you requested as I believed it would make it easier to review. I am happy to squash some of them back if you wish
And finally, I believe there are some cleanups I believe we can do later on on the pprint module (adding types, cleaning up data structures). Is that something you would like or would you rather keep it closer to upstream? I can make a PR once both this one and #11571 are done if you wish