ggplot2 icon indicating copy to clipboard operation
ggplot2 copied to clipboard

Release v3.4.0

Open thomasp85 opened this issue 2 years ago • 26 comments

Hi team @clauswilke @yutannihilation @paleolimbot @karawoo @hadley

I'm beginning to get the next release in shape. Once again this release will not be feature heavy. It will however focus on getting a lot of the internals in line as well as improving the API (with the potential of it causing some breaking changes)

I've updated to the stale 3.4.0 milestone, removing issues I don't think fit in. If any of you have something you want to work on and get into this release, please add it to the milestone so we can keep track of it

There is no release date yet but I would like to get this done at least before the end of summer

thomasp85 avatar May 20 '22 09:05 thomasp85

I think it could be nice to finally pull the trigger on switching to vctrs. @Hadley made some experimentation for this some time ago in #4342 - do you have any recollection?

thomasp85 avatar May 20 '22 09:05 thomasp85

I think I can cover the two issues that are assigned to me in the 3.4.0 milestone. I should be able to do that within the next few days.

clauswilke avatar May 20 '22 15:05 clauswilke

@thomasp85 I think the vctrs PR needs quite a lot more work to be able to tell what the downstream implications are — it's possible that it has limited impacts to the user, and could be bundled in this release, but it's also possible that it will affect a decent amount of code, and we'll need to either put it in a big release or some how make it optional.

hadley avatar May 20 '22 15:05 hadley

As the previous comment on the pull request says it would be "for the next big ggplot2 release," I too thought so.

yutannihilation avatar May 21 '22 05:05 yutannihilation

Can we add https://github.com/tidyverse/ggplot2/pull/3818 to the milestones? While I'm honestly not happy with the consequence of exposing both add_ggplot() and ggplot_add(), which looks confusing, I think we should merge the pull request if we decided to go in the direction. I don't see any blockers on including this in the next release.

yutannihilation avatar May 21 '22 05:05 yutannihilation

@yutannihilation i think I have kicked the PR down the road because I ultimately don't think I want to expose it but have also failed with coming up with a concise reason for it. But I agree we should at least make a decision about it prior to this release

thomasp85 avatar May 21 '22 11:05 thomasp85

Ah, okay. Since hadley agreed on the direction (https://github.com/tidyverse/ggplot2/pull/3818#issuecomment-1076435320, and https://github.com/tidyverse/ggplot2/issues/4743#issuecomment-1066978477) and no one has raised objections so far, I thought it was the decision. Let's discuss there if needed.

yutannihilation avatar May 21 '22 12:05 yutannihilation

Any eta about when you guys are planning on releasing v3.4.0 of ggplot2? Just thinking about how supporting the linewidth thing is going to happen in my extension package

davidhodge931 avatar Jun 23 '22 00:06 davidhodge931

OK, we seem to be very close to starting release process. @clauswilke and @yutannihilation do you have any issues or PRs not tagged with the milestone you feel needs to get into this release?

thomasp85 avatar Jul 26 '22 20:07 thomasp85

@davidhodge931 I believe august is a fair aim for this release. I'll work on an article for extension package developers helping them transition

thomasp85 avatar Jul 26 '22 20:07 thomasp85

@thomasp85 I don't.

clauswilke avatar Jul 26 '22 20:07 clauswilke

Thanks, I added two.

  • I'd like to include #4917 if possible, not because it's related to #4866, but because it's a breaking change. I'm not sure if we can agree on the change, but if so, it would be a bit too long to wait for another non-patch release.
  • #4921 also looks nice to include in the release. But I think it's safe to have this in a patch release, so this is just a nice to have.

yutannihilation avatar Jul 26 '22 21:07 yutannihilation

I found #3816 could be the one that the next release is the good timing to release, but it might be a bit too late...

yutannihilation avatar Aug 13 '22 04:08 yutannihilation

I added https://github.com/tidyverse/ggplot2/issues/4953 to the milestone because this is probably what we should decide before releasing linewidth.

yutannihilation avatar Aug 21 '22 15:08 yutannihilation

Thanks @yutannihilation - let's freeze the milestone at this point then... I've been distracted by CRANs move to HTML5 but I'm back working on this release now

thomasp85 avatar Aug 23 '22 09:08 thomasp85

Hi Everyone. The milestone has cleared and I'll make a release branch and begin the release procedure. Thanks everyone for their hard work 🎉❤️

thomasp85 avatar Aug 25 '22 08:08 thomasp85

Glad that we made it! Please let me know if there's anything I can help in investigating revdeps.

yutannihilation avatar Aug 25 '22 12:08 yutannihilation

Hi Team

Some feedback from the spatial community about the size to linewidth change which will unfortunately break the visuals of many of their plots. Should we take this opportunity to change the default linewidth in geom_sf to something thinner? It seems the only reason it is as it is is so that points are not too small? The defaults for lines was designed for line charts and when drawing country borders it is quite heavy so I feel a better default in sf is possible now that it is decoupled from points

thomasp85 avatar Aug 26 '22 06:08 thomasp85

I agree a thinner default should be better for polygon edges. Let me confirm, are you suggesting to change only the default linewidth of polygon features, not that of line features?

yutannihilation avatar Aug 26 '22 14:08 yutannihilation

I always reduce the thickness of polygon edges when using geom_sf(), so yes, a thinner default seems reasonable. I never plot line features so I have no opinion on that.

clauswilke avatar Aug 26 '22 14:08 clauswilke

We can't do that unfortunately - it would have to be changing both for lines and polygons

thomasp85 avatar Aug 26 '22 14:08 thomasp85

We can't do that

Really? But, isn't the default determined base on the type of the feature...? I thought it would be not very difficult to tweak this defaults here.

https://github.com/tidyverse/ggplot2/blob/a58b48c961cb391b8646bf072b6620a0c9f3d999/R/geom-sf.R#L204

I think it's a fair bet that most of the usage of geom_sf() is to draw polygons, but I hope we can let users to draw nice lines if possible. I sometimes plot roads and rivers.

yutannihilation avatar Aug 26 '22 14:08 yutannihilation

Oh yeah, maybe I spoke too soon. Will give it a go

thomasp85 avatar Aug 26 '22 15:08 thomasp85

But thinking about it, that would mean that strokes are a different width only when using defaults which is perhaps not the most intuitive thing. Consider a user looking at the default output and thinking "I wish the borders where thicker" goes on to set the line width and now suddenly both borders and lines change

thomasp85 avatar Aug 26 '22 15:08 thomasp85

I'm not sure, but I think polygons and lines are on different layers in most cases, so that surprise should be rare.

yutannihilation avatar Aug 26 '22 16:08 yutannihilation

To me, the main reason why it's justifiable to have a different default linewidth than other polygon-based geom is that it's easier to distinguish line features and polygon edges. So, I think lines should have different default.

Anyway, I'm not very familiar with GIS things. So, please ignore me if you are confident.

yutannihilation avatar Aug 26 '22 16:08 yutannihilation

@thomasp85 Through the dependency ggplot2 --> ggnewscale --> miaViz I was informed today that ggnewscale would be archived the 17th of October due to the note regarding incompatibility to HTML5 standards. The issue is a note generated on r-devel, which look like this also found in the build report of ggplot2 on CRAN:

image

  • Does ggplot2 fix those NOTEs on CRAN?
  • Any chance ggplot2 3.4 will have hit CRAN by then?

Thanks for a comment in advance.

@antagomir @TuomasBorman

FelixErnst avatar Oct 03 '22 10:10 FelixErnst

I'm in contact with CRAN and they have given it to the end of October for release so this is surprising. I'll reach out to them

thomasp85 avatar Oct 03 '22 10:10 thomasp85

Thank you. I discussed this and made them aware of this via mail. Uwe and Kurt are in the loop on this.

FelixErnst avatar Oct 03 '22 10:10 FelixErnst

Hi Team

All revdep issues have been taken care of. It was quite a lot of work, even by ggplot2 standards, but 3.4.0 is now on track for a release by the end of October to allow for packages to respond.

As part of the revdepcheck process the RC branch has been updated quite a lot and I'll briefly go through some of the changes below:

  • The mapped_discrete class which has been ported to use vctrs is back as a non-voters class. We found that vctrs was too strict for what people were doing in their extensions.
  • All new deprecations are soft to give developers time to adjust without too much fuss
  • There are now additional gaurd-rails for the size->linewidth switch so even fewer packages are affected
  • We now use a less strict version of vec_rbind(), again because we see a bit too many failures with the strictness imposed by vctrs. The issues are all related to type inconsistency in faceting variables across layers. These situations all work again but will throw a deprecation warning
  • The rest is standard fixes to issues that cropped up with the extended testing that revdeps provide

thomasp85 avatar Oct 11 '22 11:10 thomasp85