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

Preserve class when adding uneval objects #1624

Merged
merged 3 commits into from Aug 12, 2016

Conversation

Projects
None yet
3 participants
@wch
Member

wch commented Apr 29, 2016

This fixes a bug that is the root cause of rstudio/shiny#1178. When adding an aes() object to a ggplot() object, the uneval class is inadvertently dropped. For example:

# When including aes() in the ggplot() call, class is preserved
p <- ggplot(mtcars, aes(wt, mpg))
class(p$mapping)
# [1] "uneval"


# When adding aes() in the ggplot() call, class is lost
p <- ggplot(mtcars) + aes(wt, mpg)
class(p$mapping)
# [1] "list"

This PR preserves the class of the object.

@codecov-io

This comment has been minimized.

codecov-io commented Apr 29, 2016

Current coverage is 65.54%

Merging #1624 into master will increase coverage by +<.01%

@@           master      #1624   diff @@
========================================
  Files         158        158          
  Lines        5546       5548     +2   
  Methods         0          0          
  Branches        0          0          
========================================
+ Hits         3634       3636     +2   
  Misses       1912       1912          
  Partials        0          0          
  1. File R/scale-type.R (not in diff) was modified. more
    • Misses 0
    • Partials 0
    • Hits +1

Powered by Codecov. Last updated by 59c503b...8e4b0ea

@hadley

This comment has been minimized.

Member

hadley commented Jul 28, 2016

Could you please add a unit test?

@hadley hadley modified the milestone: v2.2.0 Jul 28, 2016

@wch wch added ready and removed in progress labels Aug 12, 2016

@hadley

This comment has been minimized.

Member

hadley commented Aug 12, 2016

And a bullet to NEWS.md please?

@hadley hadley merged commit b492d55 into tidyverse:master Aug 12, 2016

3 checks passed

codecov/patch 100% of diff hit (target 100%)
Details
codecov/project Absolute coverage decreased by -32.68% but relative coverage remained the same compared to 59c503b
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hadley

This comment has been minimized.

Member

hadley commented Aug 12, 2016

Thanks!

@hadley hadley removed the ready label Aug 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment