Configure tweaks + fixes + refactoring

[Enchufa2]

It seems we were working in parallel in this. I've done some more changes though. Have a look, please.

[Enchufa2]

I've restored -lextpat and -ludunits2 in configure.ac and removed src/Makevars. The latter is automatically regenerated during the configuration process, so we don't need it in the repo.

[Enchufa2]

Done. The PR is ready for review.

[edzer]

Does the current udunits branch install on Fedora without --with- options? If not, what is the error?

[Enchufa2]

It does, but I believe my approach is more general, because it first tests the presence of udunits2.h, if missing, tests udunits2/udunits2.h and then acts accordingly. I've also done some more tweaks, reformated the error and added missing checks.

[Enchufa2]

BTW, for the record, it is safe to call all the udunits2 finalizers on a NULL pointer, because all of them (ut_free, ut_free_system...) have a check inside.

[edzer]

thanks for checking! But that should never actually happen because we use XPtr, right?

[Enchufa2]

Not with XPtr, but I'm talking about these checks that I removed.

[Enchufa2]

Anyway, I have a sketch of a redesign in progress which will encapsulate all pointers in auto-finalizing classes.

[edzer]

Than one escaped my attention: the way you reimplemented it, you could call it twice, the second time with a dangling pointer, which nobody can catch.

About redesigning: I know you like fancy things, but be aware that I am not a C++ programmer. I also can't imagine a use case that justifies the ability to have more than one units system.

[Enchufa2]

Than one escaped my attention: they way you reimplemented it, you could call it twice, the second time with a dangling pointer, which nobody can catch.

You're right, I should've assigned sys=NULL. Let me fix it.

About redesigning: I know you like fancy things, but be aware that I am not a C++ programmer. I also can't imagine a use case that justifies the ability to have more than one units system.

Borrowing Bill Deney's words, a new unit conversion may be added to one unit system but not another, for instance. Also, one may want to add user-defined types to a new empty system to avoid collisions... It could be interesting.

[Enchufa2]

Fixed in 70a364a, sorry for that one!

[billdenney]

Hello,

I was looking at this package to see if it could handle what I'd proposed previously. I have a very common use case for this feature that I was wanting again today. I'll describe it in a way that will hopefully clarify the issue:

I work with multiple chemicals simultaneously (specifically, I'm working with hospital laboratory test data). I need to be able to work with molar concentration units (e.g. "mole/L" and related SI conversions from there), mass concentration units (e.g. "mg/dL"), and sometimes historical and nonstandard units (e.g. "uIU/mL" for insulin) for many different chemical simultaneously. For example, I need to be able to work with glucose (molecular weight = 180.156 g/mole) and insulin (molecular weight = 5733.55 g/mole; 1 µIU/mL = 0.143988 pmol/L) in the same code.

What I'd like to be able to do is standardize all my units simultaneously with code like the following (not tested, typed directly into GitHub).

my_data <- data.frame(
  test_name=c("glucose", "insulin"),
  original_value=c(1, 1),
  original_units=c("mg/dL", "uIU/mL"),
  standard_units=c("mmol/L", "pmol/L"),
  stringsAsFactors=FALSE)
install_conversion_constant("mole", "gram", 180.156, system="glucose")
install_conversion_constant("mole", "gram", 5733.55, system="insulin")
install_conversion_constant("mole", "IU", 143.988, system="insulin")
my_data$original_value <- set_units(my_data$original_value, my_data$original_units, system=my_data$test_name)
my_data$standard_value <- set_units(my_data$original_value, my_data$standard_units, system=my_data$test_name)