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

Silence ggplot2 3.4.0 deprecation warnings. #36

Merged
merged 2 commits into from
Jan 15, 2024

Conversation

olivroy
Copy link
Contributor

@olivroy olivroy commented Jan 14, 2024

Addresses #27

I followed advice here:

https://www.tidyverse.org/blog/2022/08/ggplot2-3-4-0-size-to-linewidth/#the-fix

There are still some warnings about aes_string(), but that would require more work, as I think it would require updating to the latest standards of NS evaluation everywhere, in dplyr and ggplot2 code for consistency. Links for this: https://ggplot2.tidyverse.org/dev/articles/ggplot2-in-packages.html and https://dplyr.tidyverse.org/articles/programming.html

Warnings in https://smouksassi.github.io/ggquickeda/reference/geom_km.html will be silenced.

Copy link
Owner

@smouksassi smouksassi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks moving away from aes sring will be another beast I tried to apply the recommended programming with !! sym etc. i the new functions thanks again for all the edits

@olivroy
Copy link
Contributor Author

olivroy commented Jan 14, 2024

But nowadays, there is something simpler that would allow users to provide input as Non-standard eval and quoted using the {{ }} (bang bang operator).

It simplifies a lot how the input can be used by dplyr and ggplot2. I can try a proof of concept maybe after the next CRAN release to ensure that changes are digestable and don't cause unexpected breakages.

@smouksassi
Copy link
Owner

yes I have started a CRAN submission earlier today I will wait for this PR as I think I need to rework the UI and Server of the main shiny app more to make sure I define a linewidth aesthetic that is applied when applicable as opposed to the size one now applied on both lines and points

@olivroy
Copy link
Contributor Author

olivroy commented Jan 14, 2024

Sure thing. But the main goal is just that everything should work as expected, for example geom_kn(size = ) should still work but will throw a single deprecation warning instead of many.

As explained in the blog post I shared, the only problematic case may be geom_linerange().

Happy to help and to answer any questions you may have

Cheers!

@olivroy
Copy link
Contributor Author

olivroy commented Jan 14, 2024

yes I have started a CRAN submission earlier today I will wait for this PR as I think I need to rework the UI and Server of the main shiny app more to make sure I define a linewidth aesthetic that is applied when applicable as opposed to the size one now applied on both lines and points

But in the shiny server you have linesize that will be mapped to linewidth. I don't really see any input size that wants to be passed to linewidth..

@smouksassi smouksassi merged commit edadc12 into smouksassi:main Jan 15, 2024
6 checks passed
@smouksassi
Copy link
Owner

ok got it, I will open another issue after merging to add a global linewidth aesthetic input and then use it when appropriate in the app but that will require ton of effort density and km in the app do not use the global size aesthetic mapping

Copy link
Owner

@smouksassi smouksassi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@olivroy
Copy link
Contributor Author

olivroy commented Jan 15, 2024

In news maybe state that the geom_ now recognize the linewidth argument ?

@olivroy olivroy deleted the ggplot-deprec branch January 15, 2024 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants