Clarify tutorial "Customizing Matplotlib with style sheets and rcParams" #20812
Conversation
Thank you for opening your first PR into Matplotlib! If you have not heard from us in a while, please feel free to ping You can also join us on gitter for real-time discussion. For details on testing, writing docs, and our review process, please see the developer guide We strive to be a welcoming and open project. Please follow our Code of Conduct. |
Thank you for working on this! I left some comments that would be good to address but are not deal-breakers. Good to merge once the docs build cleanly! |
@@ -104,15 +120,10 @@ | |||
plt.show() | |||
|
|||
############################################################################### | |||
# .. _matplotlib-rcparams: |
tacaswell
Aug 9, 2021
Member
I think I am 👍🏻 on this re-structuring to keep 3 headings (to match the 3 numbered things at the top). I am 50% sure if you can put more than one target on a section heading, but if not the 3 places we reference it will need to be updated.
andthum
Aug 10, 2021
Author
I've updated the 3 places to reference now customizing-with-dynamic-rc-settings
# | ||
# .. _matplotlibrc-sample: | ||
# | ||
# A sample matplotlibrc file | ||
# ~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
# -------------------------- |
tacaswell
Aug 9, 2021
Member
While you are here change this section heading to point out that is the default defaults?
aesthetics of ggplot_ (a popular plotting package for R_). To use this style, | ||
just add: | ||
aesthetics of ggplot_ (a popular plotting package for R_). To use this | ||
style, just add: |
tacaswell
Aug 9, 2021
Member
style, just add: | |
style, add: |
As long as you are touching this line lets remove "just" which is at best a filler word at worst alienate readers who do not find the following code simple!
Clarify that only the rcParam that is not related to style is ignored instead of the entire style sheet.
* Include `STYLE_BLACKLIST` in the documentation of `matplotlib.style.use` in alphabetical order. * Print list of rcParams in `matplotlib.RcParams` in alphabetical order. * Fix broken links to `.style.available` in `lib/matplotlib/style/core.py`.
3. :ref:`Changing your matplotlibrc file<customizing-with-matplotlibrc-files>`. | ||
Style sheets take precedence over :file:`matplotlibrc` files and setting | ||
rcParams at runtime takes precedence over style sheets. |
@docstring.Substitution( | ||
"\n".join( | ||
sorted( | ||
set(map("- {}".format, map(str.strip, STYLE_BLACKLIST))), |
QuLogic
Aug 11, 2021
Member
STYLE_BLACKLIST
is a set
already, no need to wrap it.
set(map("- {}".format, map(str.strip, STYLE_BLACKLIST))), | |
map("- {}".format, map(str.strip, STYLE_BLACKLIST)), |
andthum
Aug 11, 2021
•
Author
I used the set
to remove possible duplicates in STYLE_BLACKLIST
. It's just a security layer in case someone puts a duplicate entry in STYLE_BLACKLIST
by accident, although this is very unlikely to happen. So, I will remove it.
Edit:
The map(str.strip, STYLE_BLACKLIST)
is also just a security layer to remove white space in front or at the end of a blacklisted style parameter. So, I guess this should be removed as well.
tacaswell
Aug 11, 2021
Member
STYLE_BLACKLIST
is already a set literal when it is defined
matplotlib/lib/matplotlib/style/core.py
Lines 44 to 49 in 586fcff
and sorted
will happily iterate over the map without having to fully materialize it first. I also agree it is reasonable to expect that it will contain only valid rcparams (which at this point none of them contain whitespace!) and not worry about doing any cleaning.
That said, I am
1. :ref:`Using style sheets<customizing-with-style-sheets>`. | ||
2. :ref:`Setting rcParams at runtime<customizing-with-dynamic-rc-settings>`. | ||
3. :ref:`Changing your matplotlibrc file<customizing-with-matplotlibrc-files>`. | ||
Style sheets take precedence over :file:`matplotlibrc` files and setting | ||
rcParams at runtime takes precedence over style sheets. |
andthum
Aug 11, 2021
Author
I was also thinking about this, but finally mirrored the order of the tutorial sections. If you prefer, I can list the items in priority order. When doing this, should I also change the section order accordingly or keep it as is?
@docstring.Substitution("\n".join(map("- {}".format, rcsetup._validators))) | ||
@docstring.Substitution( | ||
"\n".join( | ||
sorted( |
andthum
Aug 11, 2021
Author
Is it desirable to sort the list of valid rcParams alphabetically or should I remove the sorting? (Same for STYLE_BLACKLIST
)
PR Summary
Closes #20800
The tutorial Customizing Matplotlib with style sheets and rcParams was not clear about what rcParams can be set in style sheets. The blacklist of rcParams that cannot be set in style sheets is now documented in the docstring of
matplotlib.style.use
and the tutorial links to this blacklist.PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).