Skip to content

gh-115362: Add documentation to pystats output #115365

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 7 commits into from
Feb 16, 2024

Conversation

mdboom
Copy link
Contributor

@mdboom mdboom commented Feb 12, 2024

This adds the better documentation for pystats being discussed on the ideas repo to the output of summarize_stats.py.

Rather than having a separate document to cross-reference, it's easy enough to include the documentation inline in the output.

An example full output is available here

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I've wanted this so many times! All data presentation should have something like this. I take it it's not yet completed?

@mdboom
Copy link
Contributor Author

mdboom commented Feb 15, 2024

I take it it's not yet completed?

Yeah, I just wanted to wait until the content (faster-cpython/ideas#650) was done, which it (mostly) is now.

@mdboom
Copy link
Contributor Author

mdboom commented Feb 15, 2024

This now uses <details> within HTML-style tables, as @AlexWaygood suggested. It also includes all of the docs from faster-cpython/ideas#650.

An example full output is available here

@mdboom mdboom marked this pull request as ready for review February 15, 2024 17:42
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.

Thanks @mdboom! The example output looks great to me now, on mobile and desktop. I left two optional suggestions below:

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

LGTM, though you may want to apply Alex's suggestions. Thanks for doing this!

@mdboom
Copy link
Contributor Author

mdboom commented Feb 16, 2024

I've applied @AlexWaygood's suggestions. This is ready to merge, IMHO. Thanks for the review.

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.

Thanks again! I'll enable auto-merge

@AlexWaygood AlexWaygood enabled auto-merge (squash) February 16, 2024 16:26
@AlexWaygood AlexWaygood merged commit fbb0169 into python:main Feb 16, 2024
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
LukasWoodtli pushed a commit to LukasWoodtli/cpython that referenced this pull request Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants