feat(ir): add abstract ir data class and nodes #393
Conversation
glencoe
force-pushed
the
390-add-node-data-structure
branch
2 times, most recently
from
d6f4e05
to
649cae7
Compare
glencoe
requested review from
LeoBuron and
mokouMonday
and removed request for
mokouMonday and
LeoBuron
| def __init__(self, hidden_names: set[str]): | ||
| self._hidden_names = hidden_names | ||
|
|
||
| def __get__(self, instance, owner): |
There was a problem hiding this comment.
Add commentary that owner is necessary for python API and link to python docs for descriptor
| def __get__(self, instance, owner): | ||
| return _HidingDict(self._hidden_names, instance.data) | ||
|
|
||
| def __set__(self, instance, value): |
There was a problem hiding this comment.
add type hints for instance and value. potential liskov violation. Prefer raising an error
| self.data = data | ||
| self._hidden_names = set(hidden_names) | ||
|
|
||
| def __setitem__(self, key, value): |
| def _is_hidden(self, name: str) -> bool: | ||
| return name in self._hidden_names | ||
|
|
||
| def __delitem__(self, key): |
| del self.data[key] | ||
|
|
||
| def __iter__(self): | ||
| return filterfalse(self._is_hidden, iter(self.data)) |
| for k in args: | ||
| if k in set_transforms: | ||
| args[k] = set_transforms[k](args[k]) | ||
| return args |
There was a problem hiding this comment.
Either remove return value and add comment for inplace operation or explicitly save the args in line 83 (__turn_arguments_into_data_dict)
| for k in args: | ||
| if k in set_transforms: | ||
| args[k] = set_transforms[k](args[k]) | ||
| return args |
There was a problem hiding this comment.
Either remove return value and add comment for inplace operation or explicitly save the value in __turn_arguments_into_data_dict
| return kwargs | ||
|
|
||
| @classmethod | ||
| def __transform_args_with_mandatory_fields( |
| from .attribute import AttributeT | ||
|
|
||
|
|
||
| class Node(AbstractIRData): |
There was a problem hiding this comment.
Maybe split mandatory fields class in transformable mandatory fields and mandatory fields
elasticai/creator/ir/node_test.py
Outdated
| return cls._do_new(*args, **kwargs) | ||
|
|
||
| n = NewNode.new(name="my_name", type="my_type", input_shape=3) | ||
| assert n.input_shape == (3, 1) |
There was a problem hiding this comment.
Maybe add a test similar to set_transform for get_transform?
glencoe
force-pushed
the
390-add-node-data-structure
branch
from
44d9d2e
to
88e0ca9
Compare
The purpose of this class is to provide a way to easily write new wrappers around dictionaries, that let us customize access, while still supporting static type annotations.
glencoe
force-pushed
the
390-add-node-data-structure
branch
from
88e0ca9
to
bb81f0d
Compare
The purpose of this class is to provide a way to
easily write new wrappers around dictionaries,
that let us customize access, while still
supporting static type annotations.