toUnit() not converting values

[chris-domastro]

Noticed this when writing some code to convert between Foot and Meter. When using to(Meter), the code works properly, but the alternate form toMeter() only converts the unit, not the value.

import unchained

let 
  val1 = 100.Foot
  val2 = val1.to(Meter)     # Works properly
  val3 = val1.toMeter()     # Only converts unit, not value

doAssert val1 == 100.Foot
doAssert val2 == 30.48.Meter
doAssert val2 == val1
echo val3                   # prints "100 m"
doAssert val3 == val2       # Fails

[chris-domastro]

I just looked at this again and realized that there is no toMeter() being generated at all.

What is happening is that parsePrefixAndUnit in parse_units.nim is checking for a valid prefix, but no error or warning is given for an invalid/unknown prefix. Instead, the siIdentity value is used for the prefix when it is not found in parseSiPrefixShort or parseSiPrefixLong. The example above could have had val3 = val1.whateverMeter() and the output would be the same.

I'm not sure what the implications are for changing that defaulting logic though.

[Vindaar]

You had me very confused with the issue, because my first reaction was precisely "I don't remember defining to<Unit> helpers!" :rofl:

Thanks for the report, and yes. I'd just consider it a bug in the parsing logic. Will fix it later!

[Vindaar]

One thing to be consciously aware of is that using . to convert units is not intended to perform a 'safe' conversion.

In your case of course it was never your intention to use the . macro (it was just used because toMeter does not exist). The macro is of course there for things like the 100.Foot in your example. While I could make . to check whether the identifier before the . has a unit that matches (and even perform the same conversion logic as to does), it would break the convention of how Nim normally deals with type conversions (where . performs a lossy type conversion, i.e. int to float or vice versa). The amount of 'lossy' is certainly much higher in unchained, but writing generic code is much harder if I were to introduce a forceAs (or castAs) type conversion helper.

The reason being that there are use cases when being able to overwrite the conversion logic / safety checks are useful, which can work both for unchained units as well as regular Nim number types. This is used e.g. in Measuremancer (https://github.com/SciNim/Measuremancer), because for internal calculations the types are often more a nuisance (due to issues of dealing with compile time and run type type information etc.).

edit: maybe in the future I change my mind about this, who knows, and force 'correctness' for . as well. I'd do so if people were worried about subtle bugs / experienced such (hasn't been an issue for me).