Skip to content

Switch to asciiart for boxplot illustration. #19866

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 9, 2021
Merged

Switch to asciiart for boxplot illustration. #19866

merged 1 commit into from
Apr 9, 2021

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Apr 4, 2021

This makes the information also available at the terminal.

PR Summary

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@timhoffm
Copy link
Member

timhoffm commented Apr 4, 2021

From #19860 (comment)

I'm -0.1 on this because the ascii-art is only a rough visual approximation of the box and harder to read than an actual image. It's a trade-off between having the info in the ascii docstring and readability in the html output. But I won't block a PR.

Note that if you do the ascii-art, you'll likely have to use some left-side character for aligning the ascii lines. ReST messes up if any line in the code block is less indented than the first line. See e.g. https://matplotlib.org/devdocs/api/_as_gen/matplotlib.axes.Axes.barbs.html

@anntzer
Copy link
Contributor Author

anntzer commented Apr 5, 2021

fwiw I think this renders just fine on the website
bxp
but let's see what others think...
(perhaps I can use "o" instead of "." for outliers, though)

@jklymak
Copy link
Member

jklymak commented Apr 5, 2021

Personally I would remove most of this verbiage, and just point to wikipedia and an example (assuming we have one). I agree this didn't belong in the FAQ, but I don't think it really belongs in the docstring either...

@timhoffm
Copy link
Member

timhoffm commented Apr 5, 2021

I'm fine with that as well.

@anntzer
Copy link
Contributor Author

anntzer commented Apr 9, 2021

I got rid of most of the verbiage, but kept the asciiart diagram and moved it to the top. I don't think it takes too much space there?
I also harmonized "box plot" to "boxplot" in the docstring.

story645
story645 approved these changes Apr 9, 2021
View reviewed changes
Copy link
Member

@story645 story645 left a comment

Choose a reason for hiding this comment

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

I think the ASCII art is kinda brilliant - is illustrative and to the point and I think worth it here since odds are most folk won't look at wikipedia.

lib/matplotlib/axes/_axes.py Outdated

Q1-1.5IQR Q1 median Q3 Q3+1.5IQR
|-----:-----|
. |--------| : |--------| . .
Copy link
Member

Choose a reason for hiding this comment

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

agree w/ you on 'O'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"o" it is

@anntzer
Switch to asciiart for boxplot illustration.
2951083 
This makes the information also available at the terminal.
@story645 story645 merged commit 5858cad into matplotlib:master Apr 9, 2021
@anntzer anntzer deleted the ba branch April 9, 2021 16:28
@QuLogic QuLogic added this to the v3.5.0 milestone Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@story645 story645 story645 approved these changes

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
v3.5.0
Development

Successfully merging this pull request may close these issues.
5 participants
@anntzer @timhoffm @jklymak @story645 @QuLogic