Conversation
|
Another question: |
I don't think it makes sense to carry over functionality which will be removed in the next release. |
henrikjacobsenfys
left a comment
There was a problem hiding this comment.
This may be a stupid question, but shouldn't this inherit from or use EasyList? There is at least some duplication of logic.
Otherwise a handful of nitpicks and questions to consider.
| super().__init__(unique_name=unique_name, display_name=display_name) | ||
|
|
||
| self._protected_types = self._normalize_protected_types(protected_types) | ||
| self._name = name if name is not None else self.display_name |
There was a problem hiding this comment.
If display_name and name are both None, then self._name is also None. Is that desired, or should there be a fallback?
There was a problem hiding this comment.
I don't thing this is an issue. When name is None, self._name falls back to self.display_name, which in turn falls back to self.unique_name (auto-generated) in new_base.py: 83-86. So _name is never None.
| if not isinstance(new_name, str): | ||
| raise TypeError('Name must be a string') | ||
| self._name = new_name | ||
| self.display_name = new_name |
There was a problem hiding this comment.
Is that actually desired behavior? I would think that people want to keep name and display_name separate?
There was a problem hiding this comment.
This is intentional. The constructor already syncs them (if display_name is None and name is not None: display_name = name )
The setter keeps them in sync for consistency. If a user explicitly sets a different [display_name] via the constructor, the two diverge until name is set again.
This could be revisited if use cases emerge where they need to stay independent after initial construction.
EasyList is a pure typed container. It holds a flat list of The new |
|
Looks good to me now |
This is the collection list as described in #226
Rebased against the new
developbranch and due to the number of files affected, made a separate PR.For discussion look in: #226