-
Notifications
You must be signed in to change notification settings - Fork 28
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
Support for NA_units_ #163
Comments
As I was fuzzing the interface after the above error, I found that this error could also be clarified: set_units(NA_real_, NA, mode="standard")
Error in if (any(unclass(value) != 1)) warning(paste("numeric value", :
missing value where TRUE/FALSE needed |
IMO, a missing value should be valid; a missing unit... has no meaning. So I would like to get an error, but definitively a more informative one. |
(sorry, committed this in a branch; will soon be merged) |
As I'm working with more real data, I'm finding that this is a common issue for me. I will relatively often have a vector where there are instances of combined NA as both the magnitude and the units. What do you think about defining a new constant, In effect, |
Could you provide an example? Are there any instances of a non-NA number with a NA unit? Because, otherwise, we could simply support NA in |
What I'm working with right now is clinical laboratory data in a clinical study. When a test isn't done for some reason (for example if the patient didn't come in for a scheduled visit), then the value and units for the test are explicitly missing (like Immediately, I'm just trying to setup the data, and it looks like the following: d_no_missing <-
data.frame(Patient_ID=1:3,
Lab_Name=c("Glucose", "Cholesterol", "Cholesterol"),
Lab_Value=c(5, 100, 200),
Lab_Units=c("mmol/L", "mg/dL", "mg/dL"),
stringsAsFactors=FALSE)
d_missing <- d_no_missing
d_missing[2, c("Lab_Value", "Lab_Units")] <- NA
d_no_missing$Value_with_Units <-
mixed_units(d_no_missing$Lab_Value,
d_no_missing$Lab_Units,
mode="standard")
d_missing$Value_with_Units <-
mixed_units(d_missing$Lab_Value,
d_missing$Lab_Units,
mode="standard") |
I see. Then one path would be to enable missing units. It requires some rework of the functions that manage the units attribute, but everything else would work seamlessly. Another path would be to return plain NAs and make units work with them. Let's see what @edzer thinks. |
Great thought. I see the convenience for your use case, but I'm not convinced we should require for > set_units(1, km/h) * 2
2 [km/h] should actually return |
Me neither. If
I'm not convinced about this. It is quite a change. Where is the boundary between unitless and missing units? It is not clear to me. I wouldn't change the behaviour based on a fuzzy boundary just to support a corner case. Moreover, unitless is a units concept, but missing is an R concept; so unitless is more general for this topic. Therefore, I would default to In other words, for me, |
I agree that would be quite a change, and I was thinking about making this optional. This > set_units(1, km/h) * 1
1 [km/h]
> set_units(1, km/h) + 1
Error in Ops.units(set_units(1, km/h), 1) :
both operands of the expression should be "units" objects however annoys me. |
Missing implies different things. Missing means that you know, actually, that there is some unit associated with that data, but it is not known or went missing for some reason. It is quite different to me. |
Oh, and I think that the error from |
The conversation morphed a bit. I think that there are a few different items here:
The first is supported in the current library as is, and it should not change in my opinion. The second is also already supported in the current library, too, and in my opinion, it should not change. The third is not supported in the current library, and it supports several operations— mainly around the combination of mixed units. The example I gave above is my typical use case. Another example use case is I don’t think that it’s sensible to have a non-missing magnitude and an NA unit. For example, |
Automatically closed by 8c6bda8, which doesn't allow missing units, so I'm reopening this. |
I would like to add the following use case to support the implementation of something like |
Dear package maintainers, do you see any chance that this proposal be accepted sooner or later? I would see this notion of missing unit In my view it should first be clear that:
Now, say we have a vector
|
This is still open for two main reasons:
However, it's easier to gather consensus around something that works, so we could use a PR to refine the proposal. :) |
I'm coming back to this issue as I'm working to develop some additional unit conversion tools for laboratory data. I ahve a thought about implementation. What if we define a unit during package load called "NA_units_" as a new base unit: While that would not require a specific magnitude, it would be a simple way to accomplish the goal of a unit value that is NA. Would that be an acceptable method? The downside I see to the above is that it would be confusing because there would be a unit named "NA_units_" and a constant defined named "NA_units_", but it would still work as expected, as far as I can see: The next question would be: How should we handle calls to library(units)
#> udunits database from C:/Users/Bill Denney/Documents/R/win-library/4.1/units/share/udunits/udunits2.xml
install_unit(symbol="NA_units_")
NA_units_ <- set_units(NA_real_, "NA_units_", mode="standard")
set_units(1, "NA_units_")
#> 1 [NA_units_]
set_units(2, NA_units_)
#> 2 [NA_units_] Created on 2021-10-13 by the reprex package (v2.0.1) |
I just had a bug in my code, but it was hard to track down due to an unclear error.
What I just did was, in essence, the following:
I thought that I was getting the error because I wasn't using
mode="standard"
, but it really was due to theNA_character_
.I have two thoughts:
x
andvalue
options toset_units
areNA
, couldNA
be returned? (That could easily be a bad idea for several reasons, and I don't have a strong opinion if the answer is a resounding no.)Error: is.language(x) is not TRUE
error be clarified? I thought I was making it clear that I wasn't sending in a language object when I gaveNA_character_
, so the error didn't make sense to me. (Admittedly, I wasn't intentionally sending inNA_character_
.)A simple fix for option 2 above could possibly be changing the
stopifnot(is.character(x), length(x) == 1)
at the top of as_units.character tostopifnot(is.character(x), length(x) == 1, is.na(x))
.The text was updated successfully, but these errors were encountered: