Open RobertBaruch opened 1 year ago
Example. mypy is unhappy about Shape.cast
in ast.py
:
elif isinstance(obj, ShapeCastable):
new_obj = obj.as_shape()
ShapeCastable
has no as_shape
method.
If ShapeCastable
were implemented like this:
class ShapeCastable(ABC):
@abstractmethod
def as_shape() -> Shape:
...
I think static analysis would pass (and ShapeCastable
would look more Pythonic?)
Another case where static analysis gets mad: Value.cast
(according to the code) takes a Value
, int
, Enum
, or ValueCastable
.
The Slice
constructor takes a value
parameter, which I'm trying to find the type of. The code first does
n = len(value)
Then later on does
self.value = Value.cast(value)
The static analyzer gets mad because you can't take the len
of an int
or a ValueCastable
.
One particular issue is that currently, there is no good way to add typing to Amaranth values, modules, interfaces directly. So far my position was that type hints would be a nice to have, but right now it's not clear that benefits outweigh the drawbacks.
I'm not sure I understand? I'm just talking about typehinting the core code, like this:
ValueCastableT = Union["Value", int, Enum, "ValueCastable"]
class Value(metaclass=ABCMeta):
src_loc: Optional[Tuple[str, int]] = None
@staticmethod
def cast(obj: ValueCastableT) -> Union["Value", "Const"]:
"""Converts ``obj`` to an Amaranth value.
This way, if you're using an IDE that supports static analysis (e.g. vscode), it will immediately complain when you try to write code like Value.cast("aaaaa")
instead of having to wait until runtime to raise an exception.
Another benefit is static analysis of the core code itself. For example, in Signal.like
, we have the docs saying that other
is a Value
. However, the code says:
if name is not None:
new_name = str(name)
elif name_suffix is not None:
new_name = other.name + str(name_suffix)
But Value
does not have a name
attribute.
The thing is that people will want to do something like i_data: In[Signal[16]]
which you can sort of do, but then it becomes an i_data: In[Signal[TypeVar("width")]]
which is much less feasible. And I'm still not sure what to do about this.
(Otherwise you can't make e.g. FIFOInterface a type.)
Oh, I wasn't talking about going all the way like that. i_data: Signal
is good enough. As for FIFOInterface
... it's a type. Maybe not parameterized, but it's a type. My proposal is just to annotate the internals, for example:
class FIFOInterface:
def __init__(self, *, width: int, depth: int, fwft: bool):
...
So that if I try this:
w = Const(3)
d = Const(5)
x = SyncFIFO(w, d)
The IDE will immediately complain, rather than having to wait until runtime to get an exception.
I currently use type stubs + Pyright in my (unfortunately currently private) project. I can share them if you're interested. Unfortunately, I had to use recursive type definitions, which are unsupported by mypy, so that things like record layouts can be typechecked.
Off-topic remark: I'm a huge fan of #693, because Amaranth records suck badly.
Another +1 - I actually considered adding types to Glasgow, then realized it makes no sense without Amaranth first.
I think https://github.com/amaranth-lang/amaranth/issues/725#issuecomment-1297653766 is the way to go - start with simple types with liberal use of Any and no generics. That would already make mypy happy in projects that import amaranth, more fancy stuff can be added later.
What are your thoughts about adding PEP 484 / PEP 526 type hints to the amaranth.* code? I know that many functions are documented in docstrings, but this lets static analyzers have a go at validating user-written code. If you're game, I'd like to try.