search

tripartio / ale

Interpretable Machine Learning and Statistical Inference with Accumulated Local Effects (ALE)
https://tripartio.github.io/ale/
Other
3 stars 0 forks source link

Issues with upcoming ggplot2 version #2

Closed teunbrand closed 10 months ago

teunbrand commented 10 months ago

Hi there,

We have been preparing a new release of ggpot2 and during a reverse dependency check, it became apparent that the prospective ggplot2 3.5.0 would break ale.

More specifically, I think that the vignette can no longer be built due to plots being saved to disk. We generally don't recommend saving plots to disk, partially because the saved objects may contain internal code that is version dependent. While ggplot2 tries to keep the exported functions stable for users, no such promises are made about internal code. In this case, changes to the internals appear to have invalidated plots saved to disk with previous version of ggplot2.

In particular, we found this example where plots were loaded from disk:

https://github.com/Tripartio/ale/blob/8744f1045498e741e45c44fe375aff0c4d540200/vignettes/ale-ALEPlot.Rmd#L287-L291

To test the vignette with the release candidate, you can install it with the code below:

remotes::install_github("tidyverse/ggplot2", ref = remotes::github_pull("5592"))

The release of ggplot2 3.5.0 is scheduled for the 12th of Februari. The progress of the release can be tracked in https://github.com/tidyverse/ggplot2/issues/5588. We hope that this information finds you in a timely manner.

tripartio commented 10 months ago

Sorry, I just saw this very recently. I've rebuilt the ggplot objects with the 3.5.0 release candidate with the links that you gave. The code works now.

I know that it is not ideal to serialize ggplot objects, but this code is quite slow, so overall, I take the tradeoff of serializing the objects and then rebuilding them every year or so when the code breaks. I think that serialized objects give users a better experience in my case here.

Thanks so much for the alert!

tripartio commented 2 months ago

Update: the package is being rewritten for the next version to avoid the need to serialize ggplot objects. That design was really my beginner's error.

teunbrand commented 2 months ago

Great, that'll probably spare some people a headache in the future :)