Skip to content

Allow scale_{x/y}_sqrt() to have breaks at 0. #4775

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

Merged
merged 10 commits into from
May 17, 2022

Conversation

teunbrand
Copy link
Collaborator

Apologies for submitting a PR that will not work (yet). It aims to fix #980.

In anticipation of r-lib/scales#336, this PR implements (a version of) the fix suggested here that has become viable thanks to the change in {scales}.

Note that this PR will likely not pass the automated checks until the new {scales} version hits CRAN, at which point is seems a good idea to bump the required version in the description file.

@teunbrand teunbrand marked this pull request as ready for review April 14, 2022 20:41
@teunbrand
Copy link
Collaborator Author

With the release of the new {scales} version, this PR is now ready for your perusal. The failing check appears, to my untrained eyes, unrelated to the code in this PR.

@thomasp85
Copy link
Member

Can I get you to add a NEWS bullet?

@teunbrand
Copy link
Collaborator Author

Of course, apologies for not including it in the first place!

Copy link
Member

@hadley hadley left a comment

Choose a reason for hiding this comment

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

Looks good, but I think it needs an explicit test, rather than just relying on a change to an existing visual test.

NEWS.md Outdated
* `aes()` now supports the `!!!` operator in its first two arguments
(#2675). Thanks to @yutannihilation and @teunbrand for draft
implementations.

* Require rlang >= 1.0.0 (@billybarc, #4797)


Copy link
Member

Choose a reason for hiding this comment

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

Could you please remove this spurious diff?

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.

sqrt_trans, scale limit expansion, and missing breaks
3 participants