Skip to content

Handling of "next color in cycle" should be handled differently #5577

Closed

Description

@mdboom

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.

Activity

  1. added this to the milestone on Nov 27, 2015
  2. efiring commented on Nov 27, 2015

    @efiring
    Member

    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".

  3. mdboom commented on Nov 27, 2015

    @mdboom
    MemberAuthor

    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.

  4. tacaswell commented on Nov 27, 2015

    @tacaswell
    Member

    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:

    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".

    —
    Reply to this email directly or view it on GitHub
    #5577 (comment)
    .

  5. mdboom commented on Nov 27, 2015

    @mdboom
    MemberAuthor

    @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.

  6. mdboom commented on Nov 27, 2015

    @mdboom
    MemberAuthor

    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:

  7. efiring commented on Nov 27, 2015

    @efiring
    Member

    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.

  8. mdboom commented on Nov 27, 2015

    @mdboom
    MemberAuthor

    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.

  9. efiring commented on Nov 28, 2015

    @efiring
    Member

    "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?

  10. mdboom commented on Nov 28, 2015

    @mdboom
    MemberAuthor

    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.

  11. WeatherGod commented on Nov 30, 2015

    @WeatherGod
    Member

    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...

  12. added 2 commits that reference this issue on Dec 14, 2015
    c32f05d
    cebbe40
  13. added a commit that references this issue on Apr 11, 2016
    684dbf6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions