astropy: Compound units are not valid type annotations

Description

When type hinting with units, mypy errors on any unit that is ‘compound’.

Expected behavior

Compound units are a valid type hint.

Actual behavior

mypy errors with

type_hint.py:9: error: Invalid type comment or annotation
Found 1 error in 1 file (checked 1 source file)

Steps to Reproduce

Save code below to a file
mypy test.py

import astropy.units as u
import numpy as np

# This annotation is fine
def square_area(size: u.m):
    return size**2

# This annotation is not valid
def square_size(area: u.m**2):
    return np.sqrt(area)

System Details

macOS-12.6.1-arm64-arm-64bit Python 3.10.8 | packaged by conda-forge | (main, Nov 22 2022, 08:25:29) [Clang 14.0.6 ] Numpy 1.23.4 pyerfa 2.0.0.1 astropy 5.1.1 Scipy 1.9.3 Matplotlib 3.6.2 mypy 0.982 (compiled: yes)

About this issue

Original URL
State: open
Created 2 years ago
Comments: 16 (16 by maintainers)

Most upvoted comments

Yes, we got stopped a bit in the middle, but indeed there is no reason not to start using the way – or at least describe it!

mhvk on Dec 7, 2022

Thanks for suggesting the u.Quantity[u.m] sytnax, I hadn’t come across it before. Do you think it would be worth updating the docs to use this syntax instead of just u.m so users are exposed to this way of typing that plays nicely with mypy? For example, in the examples at https://docs.astropy.org/en/stable/api/astropy.units.quantity_input.html the typing is using the unit instance and not u.Quantity.

dstansby on Dec 7, 2022

Right now, for units, the idea we’re trying to move to (led by @nstarman) is that we would type something like what you have with Quantity[u.m**2] – which I think is cleaner since it makes clear that a Quantity with area units is expected.

Yes, that’s the intended direction. Our current problem is that for this to be a proper type annotation we would need to make a Mypy plugin to treat instances of Unit as the class itself and attach the unit instance as metadata (in the form of an Annotation) to the Quantity type. Also, we want variadic typing, which likewise requires a custom mypy plugin since variadic types are not supported out of the box. We could do it all, I think I know how, but the scale of the problem, requiring an APE, and the time commitment on my end have thus far proven too large.

Also, this shouldn’t be valid since an annotation with the unit should only ever be valid if the parameter is a subclass of that unit.

# This annotation is fine
def square_area(size: u.m):
    return size**2

Typing, except in the cases of Literals, works with classes, not instances. I think if mypy isn’t complaining then it’s because the annotation is being treated as a string (if from __future__ import annotations) or the mypy settings aren’t strict enough.

If the following is broken, I can take a look at a bug fix.

# This annotation is fine
def square_area(size: u.Quantity[u.m]):
    return size**2

nstarman on Dec 7, 2022

Typing was discussed at the Coordination meeting, and in short the conclusion was that typing deserves an APE. There is no point in opening any issues about typing before such an APE is accepted.

eerovaher on Dec 7, 2022

Maybe this is a feature request then 😉 - even if astropy doesn’t 100% work with typing, it would be good for unit type hints to at least not error with mypy.

dstansby on Dec 7, 2022