You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
scatter edgecolor is broken in Matplotlib 2.0.0b3 #6844
The documented kwarg name is edgecolors (plural)--but the actual problem is that linewidths is 0. The default is being delegated to the PatchCollection, which is taking it from rcParams['patch.linewidth']. That value is None, which for a filled patch is leading to a value of 0 in the set_linewidth() method. (We can't decide between singular and plural...)
It looks like this was deliberate. In fact, I think that a default of zero makes sense for scatter. Drawing the boundary alters the size, and in most cases, filled markers in scatter look better with no boundary.
I feel like if a user sets edgecolors='k', then they should see a black edge without having to put in another kwarg 'cause the assumption is that setting edgecolors will yield an edge. (can this just be a patch ala if edgecolors: if not linewidth: linewidth=1...?)
Yes, something along these lines could reduce the incidence of surprise. Doing it well is not trivial, though--handling these situations where changing one parameter implies changing another tends to be a bit tricky. At the very least the docstring needs to be fixed; its description of the linewidths default is simply incorrect.
Doing it well is not trivial, though--handling these situations where changing one parameter implies changing another tends to be a bit tricky.
I get...but like I feel that a core principal of arguments should be that they do what they imply they will and edgecolor implies that there will be an edgecolor.
#5596 is the origin of this change. The basic idea of the new default is fine; I think the problem is that it is implemented via the linewidths instead of the edgecolor. The simplest solution might be to remove the logic setting linewidth to zero and instead change the default edgecolor from 'face' to 'none'. All of this goes back to the PatchCollection--it is not just a scatter matter.
This might also make more sense in a different way, too. If we were to have
an edgecolor value imply a non-zero linewidth, then you have a "source of
truth" problem. If both an edgecolor and a zero linewidth were specified
(perhaps one was from the rcParams and the other from the kwargs), then
which one do you respect?
On the other hand, wasn't the problem with setting edgecolor from 'face' to
'none' is that some of the markers don't have a face at all, like 'x'?
On Wed, Jul 27, 2016 at 8:07 AM, Thomas Robitaille <[email protected]
wrote:
The simplest solution might be to remove the logic setting linewidth to
zero and instead change the default edgecolor from 'face' to 'none'
Activity
efiring commentedon Jul 27, 2016
The documented kwarg name is
edgecolors(plural)--but the actual problem is thatlinewidthsis 0. The default is being delegated to the PatchCollection, which is taking it fromrcParams['patch.linewidth']. That value is None, which for a filled patch is leading to a value of 0 in theset_linewidth()method. (We can't decide between singular and plural...)It looks like this was deliberate. In fact, I think that a default of zero makes sense for scatter. Drawing the boundary alters the size, and in most cases, filled markers in scatter look better with no boundary.
story645 commentedon Jul 27, 2016
I feel like if a user sets edgecolors='k', then they should see a black edge without having to put in another kwarg 'cause the assumption is that setting edgecolors will yield an edge. (can this just be a patch ala
if edgecolors: if not linewidth: linewidth=1...?)efiring commentedon Jul 27, 2016
Yes, something along these lines could reduce the incidence of surprise. Doing it well is not trivial, though--handling these situations where changing one parameter implies changing another tends to be a bit tricky. At the very least the docstring needs to be fixed; its description of the
linewidthsdefault is simply incorrect.story645 commentedon Jul 27, 2016
I get...but like I feel that a core principal of arguments should be that they do what they imply they will and edgecolor implies that there will be an edgecolor.
efiring commentedon Jul 27, 2016
#5596 is the origin of this change. The basic idea of the new default is fine; I think the problem is that it is implemented via the linewidths instead of the edgecolor. The simplest solution might be to remove the logic setting linewidth to zero and instead change the default edgecolor from 'face' to 'none'. All of this goes back to the PatchCollection--it is not just a scatter matter.
astrofrog commentedon Jul 27, 2016
👍 I think this would behave more intuitively
WeatherGod commentedon Jul 27, 2016
This might also make more sense in a different way, too. If we were to have
an edgecolor value imply a non-zero linewidth, then you have a "source of
truth" problem. If both an edgecolor and a zero linewidth were specified
(perhaps one was from the rcParams and the other from the kwargs), then
which one do you respect?
On the other hand, wasn't the problem with setting edgecolor from 'face' to
'none' is that some of the markers don't have a face at all, like 'x'?
On Wed, Jul 27, 2016 at 8:07 AM, Thomas Robitaille <[email protected]
efiring commentedon Jul 27, 2016
On 2016/07/27 3:40 AM, Benjamin Root wrote:
I think this might have been a problem originally, but now non-filled
markers take their color from facecolor and ignore edgecolor.