-
Notifications
You must be signed in to change notification settings - Fork 14
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
Feature: set_channel() sets or skips NA values #40
base: main
Are you sure you want to change the base?
Conversation
Hi @zeehio Thanks for all the work you are doing here and in the ggplot2 repo. It is much appreciated. I'm currently fully occupied with preparing the next ggplot2 release so I do not have time to sit down with all the PRs right now, but will once ggplot2 has been released. Feel free to continue the work in the meantime or let it sit and we can have a talk about your proposals. Best |
Hi @thomasp85, Thanks for your reply. No worries and no hurries. I don't want to bury you under pull requests 😃 . I would be very happy to talk with you. Feel free to send me an email at [email protected] to schedule it at your convenience. Thank you and good luck with the ggplot2 release 👍 Best, |
When a replacement value is `NA`, two behaviours may be expected: - Set the colour to `NA` - Ignore the modification and leave the colour as it was Here we introduce skip_na_values. If `FALSE` (the default), when the replacement value is NA we set the colour to NA. If `TRUE`, when the replacement value is NA we leave the colour unmodified.
So I can depend on these patches on my Remotes:
33c96f3
to
8624c8c
Compare
Sorry for so many pull requests.
On top of #39 (please review them in order), this pull request deals with the behaviour of missing values in functions that modify channels.
When a replacement value is
NA
, two behaviours may be expected depending to what we understand aNA
modification to be:NA
(e.g.set_channel("black", "r", NA)
returnsNA
)set_channel("black", "r", NA)
returns"black"
)Both interpretations make sense to me.
The current specification handles the case when
colour
is missing (thena_value
is used). However it does not specify the behaviour whenvalue
is missing. Currently, in all cases but one (a bug, see below) the behaviour is to set the correspondingcolour
toNA
(the "modification is invalid" intepretation).In this pull request we introduce the
skip_na_values
argument. IfFALSE
(the default), when the replacement value isNA
we set the colour toNA
. IfTRUE
, when the replacement value isNA
we leave the colour unmodified.While working on this feature I fixed an inconsistency in how the alpha channel handled missing values. If you believe it only makes sense to keep one interpretation, please tell me and I will close this pull request and replace it with another one that:
NA
invalue
Created on 2022-09-20 with reprex v2.0.2
Thanks, and again apologies for all the extra work!