Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Unname layers #234

Merged
merged 2 commits into from
Jan 24, 2025
Merged

Unname layers #234

merged 2 commits into from
Jan 24, 2025

Conversation

teunbrand
Copy link
Contributor

Hi Daniel (or other maintainers)!

We've done a revdepcheck for the upcoming ggplot2 release and found that ggsurvfit gets stuck on layers having names.
The .extract_arguments_from_attr() function fails to correctly format its result whever x$layers is a named list.

What changes are proposed in this pull request?

A change to make .extract_arguments_from_attr() more stable by unnaming the layers.

If there is an GitHub issue associated with this pull request, please provide link.

tidyverse/ggplot2#6287


Reviewer Checklist (if item does not apply, mark as complete)

  • Ensure all package dependencies are installed by running renv::install()
  • PR branch has pulled the most recent updates from master branch. Ensure the pull request branch and your local version match and both have the latest updates from the master branch.
  • If a new function was added, function included in _pkgdown.yml
  • If a bug was fixed, a unit test was added for the bug check
  • Run pkgdown::build_site(). Check the R console for errors, and review the rendered website.
  • Overall code coverage remains >99.5%. Review coverage with withr::with_envvar(list(CI = TRUE), code = devtools::test_coverage()). Begin in a fresh R session without any packages loaded.
  • R CMD Check runs without errors, warnings, and notes
  • usethis::use_spell_check() runs with no spelling errors in documentation

When the branch is ready to be merged into master:

  • Update NEWS.md with the changes from this pull request under the heading "# ggsurvfit (development version)". If there is an issue associated with the pull request, reference it in parentheses at the end update (see NEWS.md for examples).
  • Increment the version number using usethis::use_version(which = "dev")
  • Run usethis::use_spell_check() again
  • Approve Pull Request
  • Merge the PR. Please use "Squash and merge".

@ddsjoberg
Copy link
Collaborator

Thank you @teunbrand ! Do you have a target date for the next ggplot2 release?

@teunbrand
Copy link
Contributor Author

The release date hasn't been discussed yet, but I'll let you know once I got more clarity on this!

@ddsjoberg ddsjoberg merged commit ea39733 into pharmaverse:main Jan 24, 2025
8 of 9 checks passed
@teunbrand
Copy link
Contributor Author

We discussed the release and we think before summer, so May-ish if we have to give a ballpark.

@ddsjoberg
Copy link
Collaborator

awesome, thank you @teunbrand !

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants