Remove rename_size = TRUE for sf and pointrange geoms

[thomasp85]

This PR partly reverses #4884 as the omission of rename_size = TRUE in geom_pointrange()andgeom_sf()was intentional sincesize` remains a valid aesthetic.

[yutannihilation]

Hmm, sorry, I think I'm still in confusion. Could you explain what's our strategy again?

I still feel rename_size is needed for these Geoms to avoid introducing any visual changes to existing code using size. The authors of such code can recognize the warning when they execute the same code with the next version of ggplot2 so that they can update their code. Removing rename_size means the plots are silently changed with no warnings. The downside of rename_size = TRUE is, however, users cannot specify the point size only; size will affect both point size and line width so they need to specify linewidth as well just to restore the line width to the default value.

So, in my understanding this is a trade off between preserving the old code and making the new code less verbose, and I rather support the former.

[thomasp85]

Yes - this is a breaking change for these two geoms... I believe it is worth it - size and linewidth has to be decoupled at some point and I'd rather make it now where the introduction of linewidth is in fresh memory instead of later on

[yutannihilation]

Hmm, let me review the past discussion. I think that's okay, but I don't remember if we agreed on that direction. Sorry for keeping you waiting.

[thomasp85]

tagging in @hadley, just for completeness

[yutannihilation]

As this got approved, please feel free to merge if you are in hurry.

[yutannihilation]

@thomasp85
Just for the record. Looking at #4883, which made us add rename_size = TRUE to these geoms, it seems you wrote

At this point the ambiguous geoms will not differentiate but I think we would do that down the line after proper deprecation

in the original PR. So, I think this part of the description of this PR is not quite correct. I bet that wasn't intentional, considering we agreed to postpone the braking change "after proper deprecation".

the omission of rename_size = TRUE in geom_pointrange() and geom_sf() was intentional

After all, I support your decision because the FAQ section of your blog post was convincing, and I agree that we should do it now before forgetting the plan. But, I'd appreciate if you respect the past discussions a bit more...

Anyway, thanks for your hard work!

[yutannihilation]

I apologize that my comment above was wrong. I wrote

The authors of such code can recognize the warning when they execute the same code with the next version of ggplot2

but, in reality, they never see warnings because size is a valid aesthetic. So, probably it wouldn't be that effective if we keep rename_size = TRUE for a while (It seems I already noticed it in #4886 (comment)). Sorry for the noise...

[thomasp85]

But, I'd appreciate if you respect the past discussions a bit more...

That is on me. I had forgotten this past discussion and convinced myself that this was always the plan. Apologies for changing course

[yutannihilation]

No worries. Changing course itself is not a problem. I just wanted more details to make sure this is the right move because this topic has been too complex for me to understand quickly.

[trevorld]

Do you also intend to remove size from the default_aes of geom_boxplot()?

[smouksassi]

keep in mind geom_boxplot has fatten as well similar to crossbar

ggplot2/R/geom-boxplot.r

Line 280 in ae44650

fatten = fatten,

not sure if we will be able to control both independently?

[thomasp85]

@trevorld that is a good question. boxplot is a tricky one because size have been used for outlier sizing unless outlier.size is given and since it is given by default it is seldom used that way... So, size has predominantly been used to scale the stroke and it makes sense to convert it, but we can't remove size from default_aes since it remain a valid aesthetic...

fatten in boxplot/crossbar is orthogonal to all this and will remain in effect. fatten in pointrange becomes redundant with the switch to linewidth and will be deprecated with time

[trevorld]

boxplot is a tricky one because size have been used for outlier sizing unless outlier.size is given

Actually size only affects outlier size only if the user explicitly passes outlier.size = NULL. size provides a backup if outlier.size is NULL but since outlier.size has a numeric default it is never NULL unless the user explicitly passes it a NULL. IMO you should probably just remove size from default_aes. I don't think many users are going to do linewidth = 4, size = 4, outlier.size = NULL instead of linewidth = 4, outlier.size = 4.

p <- ggplot(mpg, aes(class, hwy))
p + geom_boxplot()
p + geom_boxplot(size=4) # outliers the same size as `size = 1`, `size = 2`, `size = 3` since we are using `outlier.size` default
p + geom_boxplot(size=4, outlier.size = NULL) # now outliers are bigger