Add rcParams hist.edgecolor, hist.linewidth #12884

timhoffm · 2018-11-25T15:12:03Z

PR Summary

As discussed in #12874 and part of #12493, the mpl2 style merges all histogram bars into a single area because the patches do not have an edge by default.

One can modify the default behavior by setting the rcParams patch.edgecolor and patch.force_edgecolor. However, they are too broad in their scope and may unwantedly change other patches as well. This justifies the intoduction of a dedicated hist.edgecolor rcParam.

For now, the default is left to None so that the default matplotlib behavior is unchanged, but style authors can already make use of the param. If and when we should change the matplotlib default is left for another discussion.

PR Checklist

Has Pytest style unit tests
Code is Flake 8 compliant
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)

jklymak · 2018-11-25T15:18:50Z

Ok but where does it stop? The line widths are pretty wide by default.

Personally -1 on the visual clutter of the bar outlines.

timhoffm · 2018-11-25T17:22:11Z

Ok but where does it stop?

As long as the option handling is manageable and there's a good reason to make something configurable, I don't have a problem adding more rcParams. In particular, I would not be opposed to hist.linewidth.

Personally -1 on the visual clutter of the bar outlines.

It was mentioned in #12874 (comment) that most histogram plots seem to have visually distinct bars. Whether we like that and if we want that as the default style is a separate discussion, which I'm intentionally not going into with this PR. However, I acknowledge, that it's a common look and we should thus make it possible to achieve it using styles. This is what the PR is about.

jklymak · 2018-11-25T17:39:17Z

This seems fine, but if you are up to it, a linewidth rcParam would also make sense since that will be the next ask..

jklymak · 2018-11-25T17:43:07Z

lib/matplotlib/axes/_axes.py

@@ -6724,7 +6741,8 @@ def hist(self, x, bins=None, range=None, density=None, weights=None,
                    height = m
                patch = _barfunc(bins[:-1]+boffset, height, width,
                                 align='center', log=log,
-                                 color=c, **{bottom_kwarg: bottom})
+                                 color=c, edgecolor=ec,


This gets into the color, facecolor, edgecolor confusion. Probably should be clear in the doc of color what the color means, and it'd be nice if it was consistent w/ the other instances of this. (Yes this ambiguity predated this PR due to the fact that patch kwargs are passed through to patch).

I'll look into linewidth and the color descriptions.

It's even a bit worse since the meaning of color depends on the histtype. For histtype='bar' and histtype='stepfilled' it's the facecolor. For histtype='step' it's the edgecolor and the edgecolor is ignored. Even though, this behavior is confusing, I intentionally did not touch it in this PR to keep backward compatibility.

Fair enough but the color doc string could be made more specific. When we didn’t have an edgecolor kwarg we could assume some expertise in the part of the user who were able to drill down to the patch kwargs. But with that kwarg it’s going to lead to confusion.

Going to work in it.

Reworked the documentation of the color parameter.

timhoffm · 2018-12-01T18:45:27Z

hist.linewdith rcparam added.

jklymak · 2018-12-03T16:59:50Z

Failing py 3.5 tests for some reason 😢

timhoffm · 2018-12-03T20:29:34Z

Ok, waiting for 3.5 removal (#12358). 😄

timhoffm · 2018-12-05T23:10:28Z

Rebased after python 3.5 was removed in #12358.

anntzer · 2019-02-16T22:01:43Z

lib/matplotlib/axes/_axes.py

+        linewidth : float, optional
+            The linewidth of the bar edges. If not given, use
+            :rc:`hist.linewidth`. If that is *None*, the default
+            :rc:`patch.linewidth` is used.


is that fallback true even for histtype=step?

anntzer · 2019-02-16T22:02:00Z

lib/matplotlib/axes/_axes.py

+              *x*, there must be one color per dataset. If *None* the
+              standard line color sequence is used.
+
+              The *color* parameter is overridden if *facecolor* is given.


the facecolor argument is mentioned nowhere else I think?

anntzer · 2019-02-16T22:02:57Z

lib/matplotlib/axes/_axes.py

+            - For filled *histtype*\s ('bar', 'barstacked' and 'stepfilled'):
+
+              The facecolor of the bars. If providing multiple datasets to
+              *x*, there must be one color per dataset. If *None* the


or is it broadcasted? (dunno)

Nope, the description is correct:

if color is None: ... else: color = mcolors.to_rgba_array(color) if len(color) != nx: raise ValueError( "color kwarg must have one color per data set. %d data "

We could broadcast, but that's beyond this PR.

anntzer · 2019-02-16T22:03:20Z

lib/matplotlib/axes/_axes.py

+        edgecolor : color or array_like of colors, optional
+            The edgecolor of the bars. If providing multiple datasets to
+            *x*, you can either pass a single edgecolor for all datasets or
+            one edgecolor per dataset. If not given, use :rc:`hist.edgecolor`.


... unless histtype=step in which case it's a single color?

anntzer · 2019-02-16T22:08:33Z

lib/matplotlib/axes/_axes.py

+            The linewidth of the bar edges. If not given, use
+            :rc:`hist.linewidth`. If that is *None*, the default
+            :rc:`patch.linewidth` is used.
+


I guess kwargs (just below) Patch properties if histtype = bar but Line2D properties if histtype = step?

I think the discussion above about color/edgecolor/facecolor is quite confusing. I would suggest instead moving everything under the doc entry for **kwargs (which should read "Patch properties if histtype=bar; Line2D properties if histtype=step") and then have an additional paragraph like (haven't checked factual accuracy...)

Color specification follows the underlying artist type.
If using Patches (histtype=bar), the patch edge colors can be set using edgecolors (a single or list of colors), the patch face colors can be set using facecolors (...), and both can be simultaneously set using color (...); the defaults are ...
If using Line2D (histtype=step), ...

(and whatever's right for stepfilled too...)

timhoffm · 2019-02-21T18:25:50Z

Need some time to go through the review.

tacaswell · 2019-02-24T19:44:05Z

Punting to 3.2, please re-milestone if you disagree @timhoffm .

github-actions · 2023-05-23T01:54:34Z

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.

timhoffm added this to the v3.1 milestone Nov 25, 2018

timhoffm added the topic: rcparams label Nov 25, 2018

jklymak reviewed Nov 25, 2018

View reviewed changes

timhoffm added the status: work in progress label Nov 25, 2018

timhoffm force-pushed the rcparam-hist-edgecolor branch from d35fe71 to 1295778 Compare December 1, 2018 17:43

timhoffm changed the title ~~Add rcParam hist.edgecolor~~ Add rcParams hist.edgecolor, hist.linewidth Dec 1, 2018

timhoffm force-pushed the rcparam-hist-edgecolor branch 6 times, most recently from 3fbfc83 to 51aea55 Compare December 1, 2018 18:41

timhoffm removed the status: work in progress label Dec 1, 2018

timhoffm force-pushed the rcparam-hist-edgecolor branch from 51aea55 to 2956258 Compare December 2, 2018 09:41

Add rcParam hist.edgecolor

4ecbffb

timhoffm force-pushed the rcparam-hist-edgecolor branch from 2956258 to 4ecbffb Compare December 5, 2018 23:09

anntzer reviewed Feb 16, 2019

View reviewed changes

timhoffm added the status: work in progress label Feb 21, 2019

tacaswell modified the milestones: v3.1.0, v3.2.0 Feb 24, 2019

timhoffm modified the milestones: v3.2.0, v3.3.0 Aug 7, 2019

QuLogic modified the milestones: v3.3.0, v3.4.0 May 2, 2020

jklymak added the status: needs rebase label Jul 23, 2020

jklymak marked this pull request as draft July 23, 2020 16:10

QuLogic modified the milestones: v3.4.0, v3.5.0 Jan 21, 2021

QuLogic modified the milestones: v3.5.0, v3.6.0 Aug 23, 2021

timhoffm modified the milestones: v3.6.0, unassigned Apr 30, 2022

story645 modified the milestones: unassigned, needs sorting Oct 6, 2022

timhoffm added the New feature label Oct 7, 2022

github-actions bot added the status: inactive Marked by the “Stale” Github Action label May 23, 2023

Impaler343 mentioned this pull request Jun 4, 2024

Add support for multiple hatches, edgecolors and linewidths in histograms #28073

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add rcParams hist.edgecolor, hist.linewidth #12884

Add rcParams hist.edgecolor, hist.linewidth #12884

timhoffm commented Nov 25, 2018

jklymak commented Nov 25, 2018

timhoffm commented Nov 25, 2018 •

edited

Loading

jklymak commented Nov 25, 2018

jklymak Nov 25, 2018

timhoffm Nov 25, 2018

jklymak Nov 25, 2018

timhoffm Nov 25, 2018

timhoffm Dec 1, 2018 •

edited

Loading

timhoffm commented Dec 1, 2018

jklymak commented Dec 3, 2018

timhoffm commented Dec 3, 2018

timhoffm commented Dec 5, 2018

anntzer Feb 16, 2019

anntzer Feb 16, 2019

anntzer Feb 16, 2019

timhoffm Feb 17, 2019 •

edited

Loading

anntzer Feb 16, 2019

anntzer Feb 16, 2019

timhoffm commented Feb 21, 2019

tacaswell commented Feb 24, 2019

github-actions bot commented May 23, 2023

Add rcParams hist.edgecolor, hist.linewidth #12884

Are you sure you want to change the base?

Add rcParams hist.edgecolor, hist.linewidth #12884

Conversation

timhoffm commented Nov 25, 2018

PR Summary

PR Checklist

jklymak commented Nov 25, 2018

timhoffm commented Nov 25, 2018 • edited Loading

jklymak commented Nov 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timhoffm Dec 1, 2018 • edited Loading

Choose a reason for hiding this comment

timhoffm commented Dec 1, 2018

jklymak commented Dec 3, 2018

timhoffm commented Dec 3, 2018

timhoffm commented Dec 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timhoffm Feb 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timhoffm commented Feb 21, 2019

tacaswell commented Feb 24, 2019

github-actions bot commented May 23, 2023

timhoffm commented Nov 25, 2018 •

edited

Loading

timhoffm Dec 1, 2018 •

edited

Loading

timhoffm Feb 17, 2019 •

edited

Loading