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

update tests based on dev ggplot2 #951

Merged
merged 2 commits into from
Oct 26, 2024
Merged

update tests based on dev ggplot2 #951

merged 2 commits into from
Oct 26, 2024

Conversation

topepo
Copy link
Member

@topepo topepo commented Oct 23, 2024

The ggplot2 news file implies that the plot object structure has changed a bit when it comes to labels:

(internal) default labels are derived in ggplot_build() rather than in ggplot_add.Layer() (@teunbrand, #5894)
An attempt is made to use a variable's label attribute as default label (@teunbrand, #4631)

We have tests that inspect the labels and, if we are using the defaults, they are now missing.

This PR will probably fail until #950 is merged.

This probably affects finetune too @simonpcouch

@teunbrand
Copy link

Hi Max, thanks for keeping vigilant and up to date! If you want to continue to test the labels, we also implemented an accessor (tidyverse/ggplot2#6078) for exactly this reason.

@topepo
Copy link
Member Author

topepo commented Oct 23, 2024

we also implemented an accessor (tidyverse/ggplot2#6078) for exactly this reason.

That, good sir, is exceedingly helpful.

Is there anything similar for mappings and/or facets

@teunbrand
Copy link

For strip labels we have a new get_strip_labels() function (tidyverse/ggplot2#5924).
I don't think we have something for mappings, but I wouldn't oppose implementing something to that extent if you file a feature request.

@topepo topepo marked this pull request as ready for review October 23, 2024 20:40
@topepo topepo requested a review from hfrick October 23, 2024 20:40
@hfrick
Copy link
Member

hfrick commented Oct 25, 2024

Max, did you want to make use of that accessor that Teun mentioned? Or not at this time?

@topepo
Copy link
Member Author

topepo commented Oct 25, 2024

It's in dev ggplot2 so we'll wait #952

@topepo topepo merged commit 12faa6d into main Oct 26, 2024
12 checks passed
@topepo topepo deleted the autoplot-labels branch October 26, 2024 01:44
Copy link

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants