Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openff-units 0.2.1 breaks mypy for openff-models #69

Closed
dwhswenson opened this issue Jun 19, 2023 · 5 comments
Closed

openff-units 0.2.1 breaks mypy for openff-models #69

dwhswenson opened this issue Jun 19, 2023 · 5 comments

Comments

@dwhswenson
Copy link
Member

dwhswenson commented Jun 19, 2023

Not sure if this belongs here or on openff-models, but here's a MCVE:

# file named example.py
from openff.models.types import FloatQuantity
import openff.units  # type: ignore

print(f"{openff.units.__version__=}")
models_quantity: FloatQuantity["kelvin"]

Then, with openff-units 0.2.0 (and openff-models at current release, 0.0.5):

$ python example.py && mypy example.py
openff.units.__version__='0.2.0'
Success: no issues found in 1 source file

With openff-units 0.2.1:

$ python example.py && mypy example.py
openff.units.__version__='0.2.1'
example.py:5: error: Variable "openff.models.types.FloatQuantity" is not valid as a type  [valid-type]
example.py:5: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
example.py:5: error: Name "kelvin" is not defined  [name-defined]
Found 2 errors in 1 file (checked 1 source file)

see also OpenFreeEnergy/openfe#461

@mattwthompson
Copy link
Member

0.2.1 breaks lots of stuff for reasons that have caused me physiological aging (blood pressure, hair loss, etc.). For now the recommendation is to stick to 0.2.0, since I think that works, and we'll have fixes out as soon as we can (one attempt at #68).

@mattwthompson
Copy link
Member

I'm also not sure if this affects both Pint 0.21 and 0.22, but I'm pretty sure one of them works with 0.2.0

@mattwthompson
Copy link
Member

With #84 I have it passing on Pint 0.21, 0.22, 0.23

With openforcefield/openff-models#42 it's also passing with Pydantic models. The SomeQuantity['preferred_unit'] paradigm is likely going away, I unfortunately don't think that pattern is future-compatible with how tooling interprets string literals brackets inside of things that look like types.

#77 would be my preferred solution but that's around three years out of being compatible with OpenFF's Python support

@mattwthompson
Copy link
Member

This still sticks around on the v1 API, it should be gone with v2

$ cat example.py && python example.py && mypy example.py                                                                           14:00:02  ☁  0.2.2 ☂
# file named example.py
import openff.units
import openff.models

from openff.models.types import FloatQuantity
from pydantic.v1 import BaseModel
from openff.units import Quantity


print(f"{openff.units.__version__=}")
print(f"{openff.models.__version__=}")


class Model(BaseModel):
    temperature: FloatQuantity["kelvin"]


print(f"{Model(temperature=Quantity("419.0 kelvin")).temperature=}")
openff.units.__version__='0.2.2'
openff.models.__version__='0.1.2'
Model(temperature=Quantity("419.0 kelvin")).temperature=<Quantity(419.0, 'kelvin')>
example.py:15: error: Variable "openff.models.types.FloatQuantity" is not valid as a type  [valid-type]
example.py:15: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
example.py:15: error: Name "kelvin" is not defined  [name-defined]
Found 2 errors in 1 file (checked 1 source file)

@mattwthompson
Copy link
Member

These code paths are outdated now - closing, let us know if this needs to be revived

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants