Copying data container#335
Conversation
| self.system_variables[key] = contents | ||
|
|
||
| elif isinstance(contents, dict) and 'default' in contents: | ||
| print("optional ", key) |
| with make_temporary_directory() as temp_dir: | ||
|
|
||
| for key, class_data in yml_data['CUDS_KEYS'].items(): | ||
| print("cuds keys ", key, class_data) |
Current coverage is 93.69% (diff: 97.42%)
|
|
@mehdisadeghi ready for review |
| self._data = new_data | ||
| else: | ||
| self._data = DataContainer(new_data)''') | ||
| data = DataContainer.new_with_restricted_keys( |
There was a problem hiding this comment.
The keys shall remain flexible. All CUBA keys are accepted.
There was a problem hiding this comment.
Does this mean that the concept of supported keys is no longer valid?
There was a problem hiding this comment.
It is still valid. We want to leave it flexible, however. To the extent of CUBA keys.
There was a problem hiding this comment.
What I mean is also the method supported_keys in the generated objects. If we don't restrict, what's the point of having that method?
There was a problem hiding this comment.
It is rendered useless. People used to like it. Deprecate it if you like.
| for key, content in chain( | ||
| self.optional_user_defined.items(), | ||
| self.inherited_optional.items(), | ||
| ): |
There was a problem hiding this comment.
I see that this change in order has resulted in the changes in the generated init signatures. Is this intentional?
Just an observation: I suppose we cannot fix the signature such that future additional optional arguments would only be added at the end of a signature. Somewhere (in the doc maybe) the users should be advised to use named arguments when instantiating meta classes.
|
@kitchoi yes, it's intentional, because it was currently inconsistent. |
|
@sbo Ok! Thanks. |
|
We decided to copy everything, ignoring the supported parameters. |
|
Also, see issue #341 |
|
Everything looks good to me! |
|
Related PR #326 |
Performs copy of the data container at every interaction, as agreed with @mehdisadeghi