Automatic units conversion design

[o-murphy]

Quick API design question 🤔

Right now constructors auto-convert units:

weapon = Weapon(sight_height=2.5, twist=10)  # floats → Distance objects

This is convenient but constructors do too much. Thinking about cleaner separation.

Note: This affects not just constructors but also methods like calc_powder_sens(velocity, temp) and get_velocity_for_temp(temp) that currently auto-convert their arguments.

Options:

1. Keep as is - convenient, works, why change?

2. Factory method

   Weapon.create(sight_height=2.5)  # converts
   Weapon(Distance.Inch(2.5))       # direct

3. Keyword flag

   Weapon(sight_height=2.5)              # converts (default)
   Weapon(sight_height=distance, raw=True)  # direct

4. Separate method

   Weapon(sight_height=2.5)           # converts (current)
   Weapon.from_units(distance, ...)   # direct, no conversion

What would you prefer? Does breaking the current API (option 2-4) bother you if it means cleaner, more explicit code?

[o-murphy]

Alternative is making something like that

class AutoDimensionField:
    """Descriptor that automatically converts values through PreferredUnits."""
    
    def __init__(self, preferred_unit: str, default=None):
        self.preferred_unit = preferred_unit
        self.default = default
    
    def __set_name__(self, owner, name):
        self.private_name = f'_{name}'
    
    def __get__(self, obj, objtype=None):
        if obj is None:
            return self
        return getattr(obj, self.private_name)
    
    def __set__(self, obj, value):
        # Convert on EVERY assignment
        if value is None:
            value = self.default
        converter = getattr(PreferredUnits, self.preferred_unit)
        setattr(obj, self.private_name, converter(value))


class Ammo:
    mv = AutoDimensionField('velocity')
    powder_temp = AutoDimensionField('temperature', Temperature.Celsius(15))
    
    def __init__(self, dm, mv=None, powder_temp=None):
        self.dm = dm
        self.mv = mv  # Now ACTUALLY calls descriptor.__set__
        self.powder_temp = powder_temp

[dbookstaber]

If you really want to take the defaulting work out of the constructors this looks like a good approach.

[dbookstaber]

I assume that all constructors still accept explicit Units, and it is only when passed a float that they assume default units.

I would hesitate to break the API by removing default units. Users can (and arguably should be encouraged to) still pass explicit Units instead of floats, but I personally like the ability to write quickly and succinctly when I am comfortable with my default units.

[o-murphy]

The simple test

from dataclasses import dataclass
from typing import Union


class PreferredUnits:
    """Mock PreferredUnits for testing."""
    
    @staticmethod
    def velocity(value):
        if value is None:
            return None
        if isinstance(value, Velocity):
            return value
        return Velocity(value, "m/s")
    
    @staticmethod
    def temperature(value):
        if value is None:
            return None
        if isinstance(value, Temperature):
            return value
        return Temperature(value, "C")


class Velocity:
    def __init__(self, value: float, unit: str):
        self.value = value
        self.unit = unit
    
    def __repr__(self):
        return f"Velocity({self.value}{self.unit})"


class Temperature:
    def __init__(self, value: float, unit: str):
        self.value = value
        self.unit = unit
    
    def __repr__(self):
        return f"Temperature({self.value}°{self.unit})"


class AutoDimensionField:
    """Descriptor that automatically converts values through PreferredUnits."""
    
    def __init__(self, preferred_unit: str, default=None):
        self.preferred_unit = preferred_unit
        self.default = default
    
    def __set_name__(self, owner, name):
        self.private_name = f'_{name}'
    
    def __get__(self, obj, objtype=None):
        if obj is None:
            return self
        return getattr(obj, self.private_name)
    
    def __set__(self, obj, value):
        # Convert on EVERY assignment
        if value is None:
            value = self.default
        converter = getattr(PreferredUnits, self.preferred_unit)
        setattr(obj, self.private_name, converter(value))


class Ammo:
    """Test without dataclass."""
    
    mv = AutoDimensionField('velocity')
    powder_temp = AutoDimensionField('temperature', Temperature(15, 'C'))
    
    def __init__(self, name: str, mv=None, powder_temp=None):
        self.name = name
        self.mv = mv  # Calls descriptor.__set__
        self.powder_temp = powder_temp  # Calls descriptor.__set__


# Test
print("=== Test 1: Float values (auto-convert) ===")
ammo1 = Ammo("Test1", mv=815, powder_temp=20)
print(f"ammo1.mv = {ammo1.mv}")  # Should be Velocity(815m/s)
print(f"ammo1.powder_temp = {ammo1.powder_temp}")  # Should be Temperature(20°C)

print("\n=== Test 2: Already typed values ===")
ammo2 = Ammo("Test2", mv=Velocity(800, "fps"), powder_temp=Temperature(68, "F"))
print(f"ammo2.mv = {ammo2.mv}")  # Should remain as-is
print(f"ammo2.powder_temp = {ammo2.powder_temp}")

print("\n=== Test 3: None values (use defaults) ===")
ammo3 = Ammo("Test3")
print(f"ammo3.mv = {ammo3.mv}")  # Should be None
print(f"ammo3.powder_temp = {ammo3.powder_temp}")  # Should be Temperature(15°C)

print("\n=== Test 4: Reassignment ===")
ammo1.mv = 900
print(f"After reassignment: ammo1.mv = {ammo1.mv}")  # Should convert again

print("\n=== Test 5: Assign number to existing instance ===")
ammo1.mv = 950  # What happens here?
print(f"After number assignment: ammo1.mv = {ammo1.mv}")
print(f"Type: {type(ammo1.mv)}")

[o-murphy]

Unfortunately, this will not work, because the field descriptors are determined during the module loading and when the preffered units are changed, they will not be updated

[serhiy-yevtushenko]

For me Option 2 (Factory Method/usage of explicit units) looks the cleanest.

Perhaps had a look for ideas as well here - https://github.com/KentBeck/MoneyPython/ (here the conversion to explicit value in currency in case of different currencies is done once, when it is needed (and to explicit rate), and till that moment the money are each kept in their own currencies)

[o-murphy]

I saw the solutions like that but the best is the pint library. But otherwise it is somehow overingenered.

[o-murphy]

I have re-evaluated our current approach. Given that the global PreferredUnits configuration causes issues with state providers and reactivity—while also adding unnecessary overhead—I am considering moving away from automatic unit conversion in constructors and certain functions. Instead, I am looking toward requiring explicit types for these inputs.

This change would also simplify updating values in already initialized objects via properties (currently, direct field modification can lead to undefined behavior). I am considering adding factory methods to support the legacy PreferredUnits global approach for those who need it.

However, such a refactoring would break backward compatibility and wouldn't be implemented immediately. I would like to hear your thoughts: should we proceed with this structural change, or keep the current implementation as is?