Skip to content

Conversation

@hmgaudecker
Copy link
Contributor

@hmgaudecker hmgaudecker commented Dec 22, 2025

Lots of improvements, highlights:

  • The typing of OrigParamSpec was off. What we had written was "one of two exclusive kinds of dictionaries" where we wanted to write "One dictionaries where key/value pairs can take one of two forms". Fixed via a Protocol.
  • Use TypeAlias for OrderedQNames instead of TypeVar (which created a different instance per function / module).
  • Define RawParamValue since we were using RawParam as a type annotation in many places where RawParam.value was passed.
  • No global overrides! (Except for invalid-return-type, which is required for policy_inputs, where we annotate with the expected type, but return None)

ToDo:

  • Behavior of _fail_if_requested_nodes_cannot_be_found was incorrect and tests only passed because we happened to add the item that should fail at the end, else earlier failures would get overwritten by the loop. See demonstration in this commit. Check relevant changes: git diff 12be01be395dc4db6c6f119ff3c11d0a73dd0416~2.

    When fixing the loop and not commenting out the second clause below, we saw failures.

    Issue is that when we request only upstream stuff, input-dependent interface functions like (input_data, flat) are never being created. It is not fully obvious to me why we need that check at all.

     if missing_main_targets:  # or missing_main_targets_from_include_conditions:
  • When done with the above, remove "tests/test_main.py" = ["ERA001"] from pyproject.toml

@MImmesberger
Copy link
Collaborator

MImmesberger commented Dec 22, 2025

Issue is that when we request only upstream stuff, input-dependent interface functions like (input_data, flat) are never being created. It is not fully obvious to me why we need that check at all.

I think the idea was to check that the input-options of the input dependent interface functions (idif) are among the output of load_flat_interface_functions_and_inputs() (so before actually building the DAG).

I think this is a remnant from the times were I/we thought that idifs can be anywhere in the DAG. But then we settled on them using InterfaceInputs exclusively.

So I think we can i) remove the check, and ii) rename _fail_if_requested_nodes_cannot_be_found to _fail_if_requested_nodes_not_in_dag (took me some time to realise that).

Instead of adding a new check to main after loading all interface functions and inputs, we could also write a test that does that. We don't have many idifs and I don't think we need to check for typos in the idif definition everytime we run main.

Copy link
Collaborator

@MImmesberger MImmesberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I think this makes the type hints much more reliable, which will be incredibly useful!

@hmgaudecker hmgaudecker merged commit 8a07b10 into main Dec 23, 2025
11 checks passed
@hmgaudecker hmgaudecker deleted the ty branch December 23, 2025 14:25
hmgaudecker added a commit to ttsim-dev/gettsim that referenced this pull request Dec 23, 2025
- Changes corresponding to [TTSIM PR 63](ttsim-dev/ttsim#63).
- Most importantly, fix a bunch of cases where we were using `RawParam` in places where it should have been `RawParamValue`
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.

3 participants