Define a more strictly typed "sim_cfg" in some Deploy subclasses#153
Define a more strictly typed "sim_cfg" in some Deploy subclasses#153rswarbrick wants to merge 1 commit intolowRISC:masterfrom
Conversation
These are: CompileSim, RunTest and CovReport. The types reflect what we are already doing. Taking CompileSim as an example, the stricter type is even reflected in the type of CompileSim.new. Take a spare copy of the object, but with a tighter type, to convince the type checker that we have indeed got what we think. Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
rswarbrick
force-pushed
the
stricter-sim-cfg
branch
from
848c7fb to
1079eb9
Compare
| # Register a copy of sim_cfg which is explicitly the SimCfg type | ||
| self._typed_sim_cfg: SimCfg = sim_cfg |
There was a problem hiding this comment.
This seems reasonable - my personal preference for python is to use from __future__ import annotations and (long-term) to decouple the objects to resolve circular dependencies and fix these kinds of issues, but I guess we haven't done this for historical reasons (using older Python versions). This is a pragmatic workaround for the meantime.
Referring back to my comment on #144:
I wonder, would it not be a good idea to rename
sim_cfg->flow_cfgas well?
I think my preference would be to have the base Deploy etc. take a flow_cfg: "FlowCfg" and then have these deploys that only work for simulation flows take a sim_cfg: SimCfg. That way we can just have self.flow_cfg and perhaps self._sim_cfg instead of self._typed_sim_cfg. With the same explanatory comment above, I think that would probably help with readability.
Even better would be to use a property so it is inherently clear that one is derived from the other and we don't have duplicated references, but I think that would run into the same circular import issue for now.
2 participants
These are: CompileSim, RunTest and CovReport.
The types reflect what we are already doing. Taking CompileSim as an example, the stricter type is even reflected in the type of CompileSim.new. Take a spare copy of the object, but with a tighter type, to convince the type checker that we have indeed got what we think.