thomasp85 / farver

High Performance Colourspace Manipulation in R
https://farver.data-imaginist.com
Other
128 stars 14 forks source link

Feature: set_channel() sets or skips NA values #40

Open zeehio opened 2 years ago

zeehio commented 2 years ago

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 a NA modification to be:

Both interpretations make sense to me.

The current specification handles the case when colour is missing (the na_value is used). However it does not specify the behaviour when value is missing. Currently, in all cases but one (a bug, see below) the behaviour is to set the corresponding colour to NA (the "modification is invalid" intepretation).

In this pull request we introduce the skip_na_values argument. 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.

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:

library(farver)
set_channel("red", "alpha", NA_integer_)
#> [1] "#FF000000"
set_channel("red", "alpha", NA_real_)
#> [1] "#FF0000"

Created on 2022-09-20 with reprex v2.0.2

Thanks, and again apologies for all the extra work!

thomasp85 commented 2 years ago

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 Thomas

zeehio commented 2 years ago

Hi @thomasp85,

Thanks for your reply. No worries and no hurries. I don't want to bury you under pull requests :smiley: .

I would be very happy to talk with you. Feel free to send me an email at sergioller@gmail.com to schedule it at your convenience. Thank you and good luck with the ggplot2 release :+1:

Best, Sergio