achevrot / impunity

A Python library to check physical units
https://achevrot.github.io/impunity/
MIT License
10 stars 0 forks source link

Problems getting started with README example #51

Closed toby-coleman closed 9 hours ago

toby-coleman commented 1 month ago

First of all, thanks for creating this library. I'm currently looking for a way to check units in my code without the performance penalty that would come with using something like Pint. A static analysis solution looks ideal to me.

However I'm struggling to get up and running. In a clean Python 3.11 environment I have created a unit_check.py file based on the example in the README:

import numpy as np
from impunity import impunity

def speed(distance: "m", time: "s") -> "m/s":
    return distance / time

@impunity
def regular_conversion():
    altitudes: "ft" = np.arange(0, 1000, 100)
    duration: "mn" = 100
    result = speed(altitudes, duration)
    print(result)  # results in m/s

    result_imperial: "ft/mn" = speed(altitudes, duration)
    print(result_imperial)  # results in ft/mn

if __name__ == "__main__":
    regular_conversion()

Running this I get:

root@26508eb1c7f0:/usr/src/project/notebooks# python unit_check.py 
[0. 1. 2. 3. 4. 5. 6. 7. 8. 9.]
[0. 1. 2. 3. 4. 5. 6. 7. 8. 9.]

Should I not see converted values here? I've tried the inconsistent_units() function as well and can't get it to raise a warning or error. What am I doing wrong?

xoolive commented 1 month ago

Oh you need to add the @impunity decorator on the speed function as well.

Also, from Python 3.12, the annotations of the arguments of functions do not work as is anymore, you need to use the Annotated type. (They still work for variables in the function though). (Here float is wrong, but I am lazy ^^)

So, just fix things like this:

from impunity import impunity
from typing import Annotated

@impunity
def speed(distance: Annotated[float, "m"], time: Annotated[float, "s"]) -> Annotated[float, "m/s"]:
    return distance / time

@impunity
def regular_conversion():

and I got:

[0.      0.00508 0.01016 0.01524 0.02032 0.0254  0.03048 0.03556 0.04064
 0.04572]
[0. 1. 2. 3. 4. 5. 6. 7. 8. 9.]
xoolive commented 1 month ago

I will fix the readme :)

toby-coleman commented 1 month ago

Thanks @xoolive. Adding the @impunity decorator in Python 3.11 still did not work (error below). But changing to Python 3.12 and using the Annotated types does work! Also it reports that mn is not defined in the unit registry, updating this to min and feet/min works though.

root@26508eb1c7f0:/usr/src/project/notebooks# python unit_check.py 
Traceback (most recent call last):
  File "/usr/src/project/notebooks/unit_check.py", line 5, in <module>
    @impunity
     ^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/impunity/wrapper.py", line 262, in impunity
    return deco_f(__func)
           ^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/impunity/wrapper.py", line 128, in deco_f
    fun_tree = visitor.visit(fun_tree)  # type: ignore
               ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/impunity/visitor.py", line 177, in visit
    new_node = visitor(root)
               ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/ast.py", line 494, in generic_visit
    value = self.visit(value)
            ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/impunity/visitor.py", line 177, in visit
    new_node = visitor(root)
               ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/impunity/visitor.py", line 477, in visit_FunctionDef
    node = cast(ast.FunctionDef, self.generic_visit(node))
                                 ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/ast.py", line 494, in generic_visit
    value = self.visit(value)
            ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/impunity/visitor.py", line 177, in visit
    new_node = visitor(root)
               ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/impunity/visitor.py", line 1113, in visit_Return
    return_annotation = self.get_annotations(fun, self.current_module)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/impunity/visitor.py", line 361, in get_annotations
    annotations = {
                  ^
  File "/usr/local/lib/python3.11/site-packages/impunity/visitor.py", line 364, in <dictcomp>
    else eval(v, globals, locals)
         ^^^^^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 1, in <module>
NameError: name 'm' is not defined
xoolive commented 1 month ago

Thanks for confirming. And yes, hence the comment above

toby-coleman commented 1 month ago

One other question on the examples... should it be possible to define annotations for re-use? For example this works fine:

from typing import Annotated

import numpy as np
from impunity import impunity

meters = Annotated[float, "m"]
seconds = Annotated[float, "s"]
meters_per_second = Annotated[float, "m/s"]

@impunity
def speed(distance: meters, time: seconds) -> meters_per_second:
    return distance / time

@impunity
def conversion():
    altitudes: Annotated[float, "meters"] = np.arange(0, 1000, 100)
    duration: Annotated[float, "min"] = 100
    result = speed(altitudes, duration)
    print(result)  # results in m/s

if __name__ == "__main__":
    conversion()

But changing to this results in /usr/src/project/notebooks/unit_check.py:21 (in conversion) Function speed expected unit m but received unitless quantity

from typing import Annotated

import numpy as np
from impunity import impunity

meters = Annotated[float, "m"]
seconds = Annotated[float, "s"]
meters_per_second = Annotated[float, "m/s"]

@impunity
def speed(distance: meters, time: seconds) -> meters_per_second:
    return distance / time

@impunity
def conversion():
    # Using meters instead of Annotated[float, "m"]
    altitudes: meters = np.arange(0, 1000, 100)
    duration: Annotated[float, "min"] = 100
    result = speed(altitudes, duration)
    print(result)  # results in m/s

if __name__ == "__main__":
    conversion()
xoolive commented 1 month ago

I would try to annotate result: meters_per_second = Any thought why this shouldn't work @achevrot ?

achevrot commented 9 hours ago

@toby-coleman Sorry for the time it took me. I made some modifications in order to have your example working !

toby-coleman commented 9 hours ago

Thank you!