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
Handling of "next color in cycle" should be handled differently #5577
Currently, the code to handle fetching the "next set of properties in the cycle" is handled in _plot_process_args, which is used by methods that handle multiple plots in one command, but isn't really used much elsewhere (except in hacky ways) elsewhere.
It seems to me that if we handled this in add_line, add_patch, add_collection and friends, we could properly handle default colors everywhere. But that's an untested theory and I thought I'd raise the issue with @WeatherGod and @tacaswell who have looked at property cycling much closer than I recently.
I'm coming across this as part of the style changes where I'm taking out a lot of the hardcoded styles in the examples and discovering that many things don't handle cycles correctly that probably could.
So you are suggesting requiring this to be explicitly handled everywhere the next entry in the cycle needs to be applied? That's the sort of redundancy I'm trying to avoid. I guess we could add methods like add_line_and_apply_defaults and then change all instances of add_line to that.
The other thing we'll want to do here is have add_artists advance the cycle only once, so tools like errorbar that create a bunch of artists in the same color, could pass all its artists in at once and have the same color applied.
@tacaswell: I actually think this might be an argument in favor of not merging the add_ methods into one. We currently have independent cycles for lines and patches used by plot and fill respectively. If the methods are separate, we know which cycle to use.
It looks like add_* won't work anyway. Most artists/plotting methods set the default color internally of course. What we need is an easy API so all of them can stop doing that and get defaults from the cycler instead.
Pulling on the 2.0 style thread is certainly unraveling a great deal... :frustration emoji:
I'm afraid that the whizzy new cycler might be leading us down a rabbit hole. I think it would be wise to be very conservative about its application at this stage. Don't let the existence of this new tool drive the design.
I don't think this has much to do with the new cycler -- this bug has existed since the beginning and would be present even with the old color-only color cycle rcParam. What's bringing it to a head is the disconnect we are creating between the convenient color names and the new cycle of colors -- it's much more obvious and jarring when a particular plot type is using hard coded colors vs. the new color cycle now. But I think we have to resolve this if we really want to say we support styling in any meaningful way.
"the disconnect we are creating..." how? via the cycler?
In your initial comment for this issue, what is an example? What is the bug? Regarding the "things don't handle cycles correctly that probably could", is this really essential for the color overhaul? Is it fundamentally changing their behavior?
Is this related to #5489?
The disconnect is the fact that the default colors for many artists and plot types don't come from the color cycle. Now that the color cycle doesn't include the "stock" simple colors (r, b, g, y) etc. this is very jarring. The bug is that many artists have never used the color cycle but should. The fix is to have them pull colors from the color cycle rather than a hardcoded list of colors, or in some cases a single color. I don't think it's fundamentally changing their behavior -- just making colors work as I think is a reasonable expectation.
#5489 certainly is related to this, and would help but not fully fix the problem.
I like the idea of add_patch and friends being the place where defaults could be handled. In a sense, we could make it a policy that by the time an artist is done being added to an Axes, all of its properties should be defined, with no Nones left.
The difficulty, though, of that approach is that the logical grouping of artists is lost by this point. Consider more complex plots such as errorbars, which has multiple collections to be added, but should share the same iteration of the property cycler. There are two ways to deal with this: have a add_group() method that would take a list of artists, seek out undefined properties in all of them, grab them from the cycler if needed, and then call the respective add_* methods. That or finally implement plotting objects like we have been talking about for years... sigh...
Activity
efiring commentedon Nov 27, 2015
Logically, the "add_" methods don't seem to me to be the right place for this. I think they should just "add", not "modify and then add".
mdboom commentedon Nov 27, 2015
So you are suggesting requiring this to be explicitly handled everywhere the next entry in the cycle needs to be applied? That's the sort of redundancy I'm trying to avoid. I guess we could add methods like
add_line_and_apply_defaultsand then change all instances ofadd_lineto that.The other thing we'll want to do here is have
add_artistsadvance the cycle only once, so tools like errorbar that create a bunch of artists in the same color, could pass all its artists in at once and have the same color applied.tacaswell commentedon Nov 27, 2015
Maybe, long term remove all the add_* methods and replace them with an
add_child and add_child_and_cycle methods?
On Fri, Nov 27, 2015, 14:16 Eric Firing notifications@github.com wrote:
mdboom commentedon Nov 27, 2015
@tacaswell: I actually think this might be an argument in favor of not merging the
add_methods into one. We currently have independent cycles for lines and patches used byplotandfillrespectively. If the methods are separate, we know which cycle to use.mdboom commentedon Nov 27, 2015
It looks like
add_*won't work anyway. Most artists/plotting methods set the default color internally of course. What we need is an easy API so all of them can stop doing that and get defaults from the cycler instead.Pulling on the 2.0 style thread is certainly unraveling a great deal... :frustration emoji:
efiring commentedon Nov 27, 2015
I'm afraid that the whizzy new cycler might be leading us down a rabbit hole. I think it would be wise to be very conservative about its application at this stage. Don't let the existence of this new tool drive the design.
mdboom commentedon Nov 27, 2015
I don't think this has much to do with the new cycler -- this bug has existed since the beginning and would be present even with the old color-only color cycle rcParam. What's bringing it to a head is the disconnect we are creating between the convenient color names and the new cycle of colors -- it's much more obvious and jarring when a particular plot type is using hard coded colors vs. the new color cycle now. But I think we have to resolve this if we really want to say we support styling in any meaningful way.
efiring commentedon Nov 28, 2015
"the disconnect we are creating..." how? via the cycler?
In your initial comment for this issue, what is an example? What is the bug? Regarding the "things don't handle cycles correctly that probably could", is this really essential for the color overhaul? Is it fundamentally changing their behavior?
Is this related to #5489?
mdboom commentedon Nov 28, 2015
The disconnect is the fact that the default colors for many artists and plot types don't come from the color cycle. Now that the color cycle doesn't include the "stock" simple colors (r, b, g, y) etc. this is very jarring. The bug is that many artists have never used the color cycle but should. The fix is to have them pull colors from the color cycle rather than a hardcoded list of colors, or in some cases a single color. I don't think it's fundamentally changing their behavior -- just making colors work as I think is a reasonable expectation.
#5489 certainly is related to this, and would help but not fully fix the problem.
WeatherGod commentedon Nov 30, 2015
I like the idea of
add_patchand friends being the place where defaults could be handled. In a sense, we could make it a policy that by the time an artist is done being added to an Axes, all of its properties should be defined, with no Nones left.The difficulty, though, of that approach is that the logical grouping of artists is lost by this point. Consider more complex plots such as errorbars, which has multiple collections to be added, but should share the same iteration of the property cycler. There are two ways to deal with this: have a add_group() method that would take a list of artists, seek out undefined properties in all of them, grab them from the cycler if needed, and then call the respective add_* methods. That or finally implement plotting objects like we have been talking about for years... sigh...
Fix matplotlib#5577: Use the color cycle in more places
Fix matplotlib#5577: Use the color cycle in more places
Fix matplotlib#5577, Fix matplotlib#5489: Color cycle handling