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

Type Annotation Checker Problems with UFloat #178

Open
Xraydylan opened this issue Dec 12, 2023 · 6 comments
Open

Type Annotation Checker Problems with UFloat #178

Xraydylan opened this issue Dec 12, 2023 · 6 comments

Comments

@Xraydylan
Copy link

I am having problems with the type checking with "UFloat" (the "AffineScalarFunc" type)

In the documentation it says the following:

# Nicer name, for users: isinstance(ufloat(...), UFloat) is
# True. Also: isinstance(..., UFloat) is the test for "is this a
# number with uncertainties from the uncertainties package?":
UFloat = AffineScalarFunc

So when using "UFloat" to denote that a variable belongs is a number with uncertainties the type checker flags mathematical operations.

from uncertainties import ufloat, UFloat

value: UFloat = ufloat(1, 0.1)

result = value * 2
result2 = value * 2.2
result3 = value * value

Is this intended behavior?

Should I rather use "ufloat" when doing type annotations and use "UFloat" only for isinstance(...) checks?

It would be useful if "UFloat" could be used for type annotations.

@lebigot
Copy link
Collaborator

lebigot commented Dec 12, 2023

UFloat is indeed the correct type.

The type checker could realize that this type defines mathematical operations and avoid raising any warning for the first mathematical operation, at least.

What warnings are you getting? What type checker are you using?

@Xraydylan
Copy link
Author

I am getting this issue in my current IDE pycharm.

I am getting the following message:
image

@lebigot
Copy link
Collaborator

lebigot commented Dec 13, 2023

This looks like a limitation of PyCharm: there is enough information available (namely that UFloats can be multiplied by something) for not getting a warning.

Now, maybe it's possible to help PyCharm by adding type annotations to uncertainties? I would encourage to report this problem to PyCharm and report back here, so as to know what uncertainties could do to help.

@TimAllen576
Copy link

TimAllen576 commented Dec 14, 2023

PyCharm also gives what appears to be a much more useful error. "Class 'AffineScalarFunc' does not define '__mul__', so the '*' operator cannot be used on its instances". Also worth noting is that PyCharms type annotations do occasionally mess up eg. with np.atleast_nd on objects or arrays of objects

@lebigot
Copy link
Collaborator

lebigot commented Dec 14, 2023

Thanks for the details.

PyCharm is actually wrong about __mul__ not being defined:

In [7]: from uncertainties.core import AffineScalarFunc

In [8]: AffineScalarFunc.__mul__
Out[8]: <function uncertainties.core.wrap.<locals>.f_with_affine_output(*args, **kwargs)>

The reason is that __mul__ is built dynamically when the module is imported, and it requires some intelligence to know that it is, if just looking at the code.

Now, a solution maybe is to have PyCharm actually import uncertainties: it would see that __mul__ is defined.

I looked at numeric abstract classes, hoping I could explicitly declare that __mul__ is defined, but the numeric classes don't fit the (more limited) operations that make sense with numbers with uncertainties (such as converting to a float, which doesn't make sense for a number with uncertainty).

Now, it would be possible to define a dummy (no-op) __mul__ (and more) method for UFloat, that would be replaced dynamically when importing the module. This way PyCharm would be happier. Now, for this to be helpful, the type of the result should also be in the type annotation of mul (otherwise, all subsequent operations would not be type-checked by PyCharm, I guess), and many other functions (including mathematical operations from uncertainties.umath). This is a more serious project, that has only for goal to make up for the fact that PyCharm is apparently not doing any import of uncertainties but relies on simple, static source code analysis. I'd say that it's better to see if the type checker of PyCharm can at least import uncertainties so as to know that __mul__ is actually defined.

What do you think?

@TimAllen576
Copy link

Yep, PyCharms type checker is indeed just missing this. It would be a very large amount of effort to create dummys for all methods but given that type hints can just be left out for code like this probably makes it not a priority in my opinion.

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

3 participants