Skip to content

Fix sec.axis() when used with tidyeval (#2788) #2797

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 2 commits into from
Aug 6, 2018

Conversation

dpseidel
Copy link
Collaborator

@dpseidel dpseidel commented Aug 2, 2018

This PR make a small change to the evaluation of the transform formula of secondary axes to support plots created with tidy evaluation when calling sec_axis(). Fixes #2788.

This PR solves the original issue, and does not break any other secondary axis transformations, but if there is a better way to implement this or something this might break downstream that I'm not seeing, please advise.

@dpseidel dpseidel requested review from hadley and thomasp85 August 2, 2018 20:35
@@ -46,3 +46,29 @@ test_that("sec axis works with skewed transform", {
breaks = derive()))
)
})

test_that("sec axis works with tidy eval",{
Copy link
Member

Choose a reason for hiding this comment

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

Can you fix the style here please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

@dpseidel dpseidel merged commit d9f2cd4 into tidyverse:master Aug 6, 2018
@dpseidel dpseidel deleted the tidy_secaxis branch August 8, 2018 17:09
@lock
Copy link

lock bot commented Feb 4, 2019

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

@lock lock bot locked and limited conversation to collaborators Feb 4, 2019
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.

2 participants