Skip to content

Add type hints to edalize#525

Open
ThVerg wants to merge 12 commits into
olofk:mainfrom
ThVerg:add-type-hints
Open

Add type hints to edalize#525
ThVerg wants to merge 12 commits into
olofk:mainfrom
ThVerg:add-type-hints

Conversation

@ThVerg
Copy link
Copy Markdown

@ThVerg ThVerg commented May 13, 2026

Adds type hints across the edalize codebase. Closes #478.

ThVerg added 12 commits May 11, 2026 16:32
Addresses olofk#478.

* New module edalize/edam.py with TypedDicts modelling the EDAM dict
  (the FuseSoC <-> Edalize contract): Edam, File, Parameter, HookScript,
  Hooks, VpiModule, plus Literal aliases (ParamType, DataType, HookName).
* PEP 561 marker (edalize/py.typed) so downstream packages see the
  annotations.
* mypy configuration in pyproject.toml with lenient defaults for
  incremental adoption and stricter overrides on the three base classes
  and edam.py.
* Added typing_extensions>=4.6 runtime dep for Python <3.11 (NotRequired).
* Hand-annotated the three base classes: edatool.py, tools/edatool.py,
  flows/edaflow.py.
* Annotated ~75 backends across edalize/, edalize/tools/, edalize/flows/.
* Annotated utils.py, build_runners/make.py and the *_reporting.py modules.
* Added 'from __future__ import annotations' to 79/83 files.
* Added tests/test_type_hints_regressions.py with 274 hand-written tests
  that probe annotation-driven behaviour changes; all pass.

Verified state:
  mypy: 0 errors across 83 source files
  pytest: 474 passing (200 upstream + 274 new regression tests)
* DataType now includes 'real' to mirror FuseSoC's CAPI2 schema.
* Parameter.default widens to include float so real-valued defaults type-check.
* Edam.name is Required (Edatool.__init__ indexes it unconditionally).
* Add FuseSoC-emitted top-level keys: version, cores, dependencies, filters.
* Replace ToolDoc = Dict[str, Any] with a proper TypedDict and a
  ToolDocEntry row TypedDict; unroll the dynamic-key get_doc loop so
  TypedDict literal-key access type-checks.
* Annotate the six backends that compose ToolDoc from inline dict
  literals: apicula, gatemate, icestorm, mistral, oxide, symbiflow.

mypy: 0 errors across 83 source files
pytest: 474 passing
* self.edam: Edam | None -> Edam (narrowed after eda_api fallback, with
  a type: ignore where upstream test_empty_edam intentionally relies on
  TypeError raised by indexing None).
* self.files: list[Any] -> list[EdamFile]. This would have caught the
  tools/vpr.py f.name typo at type-check time.
* edalize.tools.edatool.setup() now also types self.files: list[EdamFile].
* flows.edaflow.Node: tool: str | None = None -> tool: str (required).
  tool.capitalize() is called unconditionally; the sole caller already
  passes a non-None string. Reordered parameters so tool comes before
  the mutable defaults.

mypy: 0 errors
pytest: 474 passing (200 upstream + 274 regression)
Codex review noted that several class-level mutable defaults and
silent-no-op changes I added during the first typing pass amount to
API drift, not annotations. Revert each to match pristine behaviour
while keeping the type information that drove the changes:

* Edatool.argtypes: list[str] (annotation only, no shared list default).
* Edatool.tool_options: dict[str, Any] (annotation only).
* Nextpnr.flow_config: dict[str, str] (annotation only).
* EdaCommands.default_target: str (annotation only).
* _apply_parameters no longer guards against None; signature is now
  RunArgs (the caller is expected to never pass None, matching main).
* Edaflow.configure_flow is hidden behind 'if TYPE_CHECKING:' so the
  attribute is documented for static checkers but does not exist at
  runtime; subclasses that forget to override still hit AttributeError.

Regression tests rewritten to assert the restored pristine behaviour
(AttributeError instead of NotImplementedError / clean RuntimeError /
empty default).

mypy: 0 errors
pytest: 474 passing
Codex pointed out that FlowGraph.fromdict(d: dict[str, Any]) was loose
enough to let the 'ftdo' misspelling in flows/vpr.py slip past static
checks. Add a FlowNodeSpec TypedDict modelling the {deps, fdto, tool}
keys and narrow:

* FlowGraph.fromdict signature
* the _flow class attribute on apicula / icestorm / trellis
* the local flow dict in apicula, generic, gls, icestorm, trellis,
  vivado, vpr

Apply the 'ftdo' -> 'fdto' fix in flows/vpr.py to match the standalone
upstream PR olofk#519 so this branch type-checks; will rebase cleanly once
the upstream PR lands.

mypy: 0 errors
pytest: 474 passing
Codex pointed out that the override on the four 'strict' modules was
just two flags, not the full strict bundle. Fix:

* edalize.edam, edalize.tools.edatool, edalize.flows.edaflow now pass
  mypy --strict cleanly. The override sets every strict-flag explicitly.
* edalize.edatool is kept on a lighter override (disallow_incomplete_defs
  + warn_return_any) because making it fully strict requires changing
  its constructor signature (Edam | None, str | None), which upstream
  test_empty_edam pins. ~22 strict errors remain there, tracked as the
  next module to tighten.
* Fix three real strict-mode issues found along the way:
  - subprocess_run_3_9 was missing param/return annotations.
  - Asserted Popen.poll() returncode after the process has exited.
  - Removed two unused-type-ignore comments revealed by --strict.

mypy (project config): 0 errors
mypy --strict (3 modules): 0 errors
pytest: 474 passing
* Apply 5 upstream bug fixes locally (mirrors the standalone PRs so the
  branch is self-contained and free of pre-existing-bug type-ignores):
    - flows/f4pga.py: return FlowGraph.fromdict({...}), not a list.
    - tools/surelog.py: append to verilog_defines, not verilog_params.
    - tools/vivado.py: build() returns the local args list.
    - tools/vpr.py: build() returns [], not undefined self.args.
    - openfpga.py: super().__init__ forwards eda_api before verbose.
* Tighten three TypedDicts that were total=False where the keys are
  indexed unconditionally:
    - HookScript.name / cmd: Required
    - ToolDocEntry.name / type / desc: Required
    - ToolDoc.description: Required
  Six legacy backends that build an accumulator dict now include a
  placeholder description (the final return statement overrides it).
* Edaflow.configure_flow: bring back the runtime
  raise NotImplementedError instead of hiding behind TYPE_CHECKING.
  The behaviour is a strict upgrade from pristine's AttributeError.
* tools/nextpnr.py: type output_files: list[EdamFile] and drop the
  type-ignore on self.edam['files'] += output_files.
* Documented note on Node default-arg anti-pattern: upstream golden
  files actually pin the leaky shared-state behaviour, so the fix
  belongs in a separate cleanup PR.

mypy: 0 errors
mypy --strict on edam, tools.edatool, flows.edaflow: 0 errors
pytest: 474 passing
Address each of the six error groups Codex identified, bringing
edalize.edatool to mypy --strict cleanliness:

  Group 1: assert retcode after Popen.poll() in subprocess_run_3_9.
  Group 2: explicit 'if edam is None: raise TypeError(...)' after the
           eda_api fallback. Preserves upstream test_empty_edam while
           narrowing self.edam to non-Optional.
  Group 3: keep work_root: str | None at the API boundary (upstream
           tests construct backends without one) but route the strict
           errors via type:ignore on the env-dict assignment and assert
           in render_template / _run_tool.
  Group 4: assert __spec__ is not None and __spec__.parent is not None
           around the importlib metadata lookup.
  Group 5: guard every get_doc(0) call site:
             - _extend_options returns early if get_doc returns None.
             - parse_args / _apply_parameters fall back to an empty
               ToolDoc shape.
             - gen_tool_docs skips backends whose get_doc returns None.
  Group 6: run_pre lets parsed_args flow as Optional with a single
           targeted type:ignore — the existing AttributeError path on
           run_pre(None) is intentional.

Promoted edalize.edatool into the fully-strict overrides block, so
all four base modules now require --strict cleanliness on each commit.

mypy --strict on edalize.edam, edalize.edatool, edalize.tools.edatool,
edalize.flows.edaflow: 0 errors
mypy (project config): 0 errors across 83 source files
pytest: 474 passing
* Remove allow_redefinition = true from the project mypy config. It
  was masking real strict-mode errors in edatool.py and tools/vpr.py.
  Fixed the three uncovered redefinitions explicitly:
    - edatool.parse_args: annotate the parse_args 'default' local as Any.
    - tools/vpr.setup: type 'depends' as str | list[str], assert before
      passing as a single dep.
    - ise_reporting._parse_twr_stats: annotate pp_units as
      pp.ParserElement so the | reassignment widens cleanly.
* Restore Node(__init__) positional ordering to pristine
  (name, deps=[], fdto={}, tool=None) so positional callers don't break.
  Validate tool with an assert inside the body; pristine crashes on
  None at .capitalize() anyway, this gives a clearer message.
* Apply the sandpipersaas depends=[] fix locally (mirrors PR olofk#520) and
  drop the silenced type:ignore.
* Apply the flows/vpr.py build_tool_graph dead-method removal locally
  (the local-only branch that I held back at the user's request) and
  drop the silenced type:ignore.

mypy: 0 errors
mypy --strict on the four strict modules: 0 errors
pytest: 474 passing
Schema:
* File.name is now Required[str]; the other keys are NotRequired.
  FuseSoC always emits 'name' and every backend indexes f['name']
  unconditionally.
* Parameter.datatype and paramtype are Required (CAPI2 rejects either
  missing); default and description stay NotRequired.
* VpiModule.name, src_files, include_dirs, libs are all Required —
  FuseSoC emits all four when it emits a VPI entry and several
  backends index them directly.

Failure-mode preservation (per Codex round 3):
* _extend_options now raises RuntimeError when get_doc(0) returns None
  instead of silently dropping the contribution. Matches pristine's
  loud failure (it crashed at help['members'] in that case).
* parse_args / _apply_parameters raise RuntimeError on the same None
  case instead of treating it as 'no backend args'.

Ignore cleanup:
* verilator.py: drop the unused type:ignore[operator] on the toplevel
  concatenation (self.toplevel: Any already suppresses the error).
* flows/edaflow.py: replace the hooks-assignment type:ignore with a
  proper cast to dict[HookName, list[HookScript]], and type
  add_scripts(hook_name: HookName).
* edatool.py run_pre: replace the type:ignore on _apply_parameters
  with cast(RunArgs, parsed_args). Same AttributeError on None path.

mypy: 0 errors
mypy --strict on the four base modules: 0 errors
pytest: 474 passing
* gen_tool_docs now raises RuntimeError when get_doc(0) returns None,
  matching the loud-fail pattern used by _extend_options / parse_args /
  _apply_parameters. The previous 'silently skip the section' was the
  one remaining silent-drop Codex flagged.

* Two upstream test fixtures used 'tags': 'simulation' (string) where
  File.tags is now typed list[str]. Runtime substring-matching
  tolerated the bug; the tests still passed only because
  '"simulation" in "simulation"' is True. Fix to ['simulation'] in
  tests/test_tool_yosys.py and tests/test_tool_vivado.py to match the
  schema and the obvious intent.

* tools/gowin.py: type _handle_src / _handle_tcl / src_file_filter /
  _append_library with EdamFile instead of Any. Removes the last two
  'returning Any as str' strict errors in edalize.tools.

* sandpipersaas.py: drop the pre-existing no-op .format(...) call on
  the empty OUTPUTDIR line. The format had no placeholder so the
  argument was always silently dropped; the rewrite removes the
  type:ignore and the dead code.

Remaining type:ignore comments are down to two, both pinned by
unreachable Python <3.7 dead code (the TimeoutExpired/_mswindows
fallback in flows/edaflow.py and edalize/edatool.py).

mypy: 0 errors
mypy --strict on edalize.edam, edalize.edatool,
edalize.tools.edatool, edalize.flows.edaflow: 0 errors
pytest: 474 passing
The subprocess_run_3_9 fallback is gated on sys.version_info < (3, 8),
so it is unreachable on Python >= 3.8, not >= 3.7 as the comment said.
Either way the branch is dead under edalize's requires-python >= 3.9.
Spotted by Codex.
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

Successfully merging this pull request may close these issues.

[Task] Add type hints to Python code

1 participant