Add: Call _check_types in init of NIRGraph #123
Conversation
There was a problem hiding this comment.
Thank you for the addition! This looks good in principle, but a lot of the tests are failing, partly, probably, because the metadata field is missing.
It could also be issues with the "enforced" type checking. That's not a bad thing, but we'll have to make sure the tests pass. I'm happy to give a hand here, but I would suggest first resolving the metadata problem and then see where we are.
nir/ir/graph.py
Outdated
| @@ -31,6 +31,12 @@ class NIRGraph(NIRNode): | |||
| output_type: Optional[Dict[str, np.ndarray]] = None | |||
| metadata: Dict[str, Any] = field(default_factory=dict) | |||
|
|
|||
| def __init__(self, nodes, edges, type_check: bool = True): | |||
There was a problem hiding this comment.
Aren't you forgetting the metadata field?
There was a problem hiding this comment.
Ah yes, you're right. I added this to the init. Does it looks fine now or should I change other things?
There was a problem hiding this comment.
Looks good, yes. A lot of other tests are failing. Let me investigate...
|
So, the changes actually exposed a lot of problems in the tests. Which is good, but it had to be fixed. Additionally, we had to correctly add the constructor arguments to I've added a PR to your repo (benkroehs#1). If you merge this into your fork, we can push it upstream. |
Fixes the `check_type` check for NIRGraph
With this change, the
_check_typesfunction is called per default during the__init__of aNIRGraph. By this, wrongly-typed graphs throw an error.(Also the function _check_types was defined twice, maybe just copied wrong)