Expected for unitful property setter without unit

[BenediktBurger]

I noticed, that a unitful property setter does not raise an error, if a number is given. In this case, it just assumes the default unit.

For a bounded_unitful_property, the same command (inst.some_unitful_property = 500) raises an error, because the comparison in unitful_property._setter happens before assume_units is called.

What is the expected behavior? Should both raise an error or should both take the magnitude and assume the default unit?
I guess the second option is desired, because assume_units is called.

I'm confident to be able to fix the issue.

Another question:
pint.Quantity(value, unit) is able to change a string like "500nm" to a meaningful quantity, but only, if unit is not given. If a unit is given, the value is the string, which will raise an error in unitful_property._setter.
I propose to accept strings and assume the unit afterwards:

def assume_units(value, units):
    if not isinstance(value, u.Quantity):  # unchanged
        value = u.Quantity(value)
        if value.check(u.Unit.dimensionless):
            value = u.Quantity(value.magnitude, units)
    return value  # unchanged

[scasagrande]

Whoops! Yeah the desired behaviour for IK has been to assume the default units if none are supplied

[scasagrande]

Accepting strings with units is an interesting idea. I'm not sure how I feel about accepting strings, and then querying the same property will return a numeric quantity. It feels wrong, but I do appreciate the idea of allowing people to set values without having to mess about with manually importing the unit registry

I can see the use case of a user just wanting to experiment with some equipment in their terminal. Passing in strings with units would be easier:

inst.property = "123units"

vs

from instruments.units import ureg as u
inst.property = 123 * u.units

While the latter is obviously preferred when writing an actual *.py file, the former can be helpful when you just want to test something quickly in the terminal.

[BenediktBurger]

I made a pull-request with the fix, tests, and also allowing strings.

I see your point in not allowing strings with units, but on the other hand, pint allows just that and allows to give the unit in form of a string (Quantity(5, "nm")).
If you prefer not allowing strings with units, we should check for it in assume_units instead of failing somewhere else.

[trappitsch]

Just found this fitting comment this morning... good timing!
https://www.nature.com/articles/d41586-022-01233-w

[scasagrande]

And they didn't reach out to us?? 🙄

[trappitsch]

And they didn't reach out to us?? roll_eyes

I know... 🤷

With respect to reading in strings with units... Sounds like a nice idea, as long as pint handles everything I don't see too much of a downside. Meaning that the the following should all be equivalent - which is the case since pint handles it all:

>>> from instruments.units import ureg as u
>>> a = u.Quantity(10, u.mm)
>>> b = 10 * u.mm
>>> c = u.Quantity("10mm")
>>> a == b == c
True

I see string parsing as the least favorable / important way of getting units in though, mostly because I rely on the fact that pint interprets "10mm" as millimeters. If I'm unsure about u.mm, I can always check the docstring, and then know it will work...
This argument might not make all that much sense, but when I use ik, I always want to load the unit registry and ensure that there's no ambiguity.