Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not plot legend upon single series #4964

Open
wants to merge 3 commits into
base: v2
Choose a base branch
from

Conversation

isentropic
Copy link
Member

@isentropic isentropic commented Jul 18, 2024

Stolen from https://github.com/JuliaPlots/Plots.jl/pull/3732/files

Basically do not insert label in case there is only single series being plotted. For me it feels like a good default. Legend would show in case you provide a custom label for a single series it of course. Having y1 to be displayed is surely weird if it is by its own

@isentropic
Copy link
Member Author

I'll fix the tests, once we agree if it goes into 2.0 or not

@BeastyBlacksmith
Copy link
Member

I'm in favor

@BeastyBlacksmith BeastyBlacksmith added this to the v2.0 milestone Jul 18, 2024
@isentropic
Copy link
Member Author

there is a small problem here, plot(rand(10), label="y1") would still suppress the label. Basically the current logic just checks if there is single series && label="y1" which is okay practically, but maybe someone really wants to display "y1" as a single series.

I propose adding a new keyword smart_label=true to have this behavior active. Otherwise I can't see how we can unhack this or return to previous behavior for compatibility reasons

@BeastyBlacksmith
Copy link
Member

if there is single series && label="y1"

Yeah, I would prefer to set the label to "" if there is only a single series or to set legend_position = :none.

@isentropic
Copy link
Member Author

#4961

@isentropic
Copy link
Member Author

isentropic commented Jul 23, 2024

I dived into the processing pipeline and how things are handled, but there doesn't seem to be a way to introduce this functionality without adding a new keyword without hacks. The problem this in this statement

Yeah, I would prefer to set the label to "" if there is only a single series or to set legend_position = :none.

The pipeline updates the legend_position before processing series. So we can't set legend_position= :none because series haven't been processed, and there is no way to know how many series are present.

Another way is to do this after series are processed. Now that all series are processed, the labels are already set to "y1","y2" and legend_position is set to :best. At this point we can't know if plots set the labels to y1,y2 or the user set it like that in the first place.

Another way might be while series are processed. But there is problem in that it is not easy to know how many more series are to come. It is kinda impossible to tell if there will be only one series in the subplot. I couldn't find a reliable way to count series while they are being processed.

A dirty solution is to make a new arg auto_labelling=false, true, :smart true would correspond to the current behavior, false would completely disable y1,y2,y3... smart will correspond to autolabelling in case there is more than one series. Having this arg, we still can't tell if the user put that y1 label there or not, but the user can enable y1 label by setting auto_labelling=true

Another solution is just to disable this auto labelling into y1,y2,y3 I'm not sure how is that even useful in the first place.

@BeastyBlacksmith
Copy link
Member

Another solution is just to disable this auto labelling into y1,y2,y3 I'm not sure how is that even useful in the first place.

Thats fine with me too

default to not put autolabelling
@isentropic
Copy link
Member Author

Well, I simplified the change to label=:none by default. So there is no longer weird numbering like y1,y2... but users can reenable it by label=:auto to restore the past behavior. Now, there is simply no label by default

@isentropic
Copy link
Member Author

Gotta say now that there is no default label, all images look much cleaner. Remind me what is the policy about making reference images. I'm on a mac, and my ref images look always different even for the ones that don't change. How should I handle this

@isentropic
Copy link
Member Author

isentropic commented Jul 25, 2024

I have a feeling that we should disable reference image checking in macos too. And maybe for the purpose of a cleaner transition maybe I should embed PlotsRefImages repo inside here the same way you wanted to that with PlotsDocs? @BeastyBlacksmith

@BeastyBlacksmith
Copy link
Member

PlotsRefImages repo

Thats pretty big. But it wouldn't be impossible to automate the creation of the ref images and opening of the pull request via CI, that would also ensure that its built on the same architecture

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants