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

Fix `rescale_mid()` to properly handle NAs. #104

merged 5 commits into from Jun 22, 2018


None yet
3 participants

foo-bar-baz-qux commented Oct 17, 2017

Noticed in the course of fixing one of the ggplot2 bugs that rescale_mid handles NAs differently than the other rescale functions. This updates rescale_mid to be inline with those other functions when handling NAs.


This comment has been minimized.


hadley commented Jun 5, 2018

Could you please add a unit test?


This comment has been minimized.


hadley commented Jun 15, 2018

@dpseidel can you please finish this one off by adding a test?

dpseidel added some commits Jun 18, 2018


hadley approved these changes Jun 19, 2018

expect_equal(rescale_mid(c(1, NA, 1)), c(0.5, NA, 0.5))

This comment has been minimized.


hadley Jun 19, 2018


I think this ok here, but it would be better to create a new test that fully exercises missing values (i.e. also checking what happens when the range isn't 0), since there are no existing tests for NAs.

This comment has been minimized.


dpseidel Jun 19, 2018


Agreed. Full tests of NA handling added in 7f7bdd5.

@foo-bar-baz-qux foo-bar-baz-qux requested a review from dpseidel as a code owner Jun 19, 2018

@dpseidel dpseidel merged commit 9e5e4d4 into r-lib:master Jun 22, 2018

3 checks passed

codecov/patch 75% of diff hit (target 64.67%)
codecov/project 64.86% (+0.18%) compared to 7152151
continuous-integration/travis-ci/pr The Travis CI build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment