Floats really really floats

[richardjgowers]

Description

This is somewhat motivated on an annoying bug we had where (e.g.) a FloatQuantity value of 1.0 * unit.nanometer was creating a different json to a value of 1 * unit.nanometer. So instead FloatQuantity should always cast to float.

Todos

Notable points that this PR has either accomplished or will accomplish.

• [ ] TODO 1

Questions

• [ ] Question1

Status

• [ ] Ready to go

[mattwthompson]

Interesting idea, I could see it being useful in plenty of situations.

creating a different json to a value

presumably this is a case where byte-by-byte equality is important?

[codecov-commenter]

Codecov Report

Merging #21 (283d86b) into main (774f42e) will decrease coverage by 0.69%. The diff coverage is 100.00%.

Additional details and impacted files

[richardjgowers]

@mattwthompson yeah we're using a hash (so byte equality matters) to check if two models are identical. Here's a minimal example:

from openff.models import types, models
from openff.units import unit

class Foo(models.BaseModel):
    a: types.FloatQuantity['1/picosecond']

# specifying the unit with an int magnitude keeps an int
f = Foo(a=1 / unit.picosecond)
f.a.m
>>> 1

# not specifying the units will give a float
Foo(a=1)
>>> Foo(a=<Quantity(1.0, '1 / picosecond')>)

[mattwthompson]

I'd be fine with a change to FloatQuantity that immediately casts all ints to floats. That would be a breaking change but as long as it's introduced alongside IntQuantity I don't see any reason to keep that wart along for the ride. If people want a quantity to be represented as a float, it's reasonable to store it as such, and if people actually want it as an int they'd be able to do that. I can't imagine a case in which something morphs between the two types in a useful way? Like, somehow it's important that it's represented as an int under some conditions and a float under others.

As far as IntQuantity goes, if/once you're happy with it I think I'm just going to ask for more tests/higher coverage. IIRC the if unit_ is Any: block is covered by models like

class Foo(models.BaseModel):
    a: types.FloatQuantity  # not defining the units

Let me know when you want this looked at or if I can help along the way

[richardjgowers]

@mattwthompson I've just included the floats fix here, so these are always floats even if you put in an int. I'll work up the Int idea separately

[mattwthompson]

@richardjgowers this is in 0.0.5 which should be online shortly