Review comments2 #7558
Review comments2 #7558
Conversation
wochinge
requested review from
a team,
tmbo and
Ghostvv
and removed request for
a team
rasa/shared/core/events.py
Outdated
|
|
||
| def __str__(self) -> Text: | ||
| """Returns text representation of event.""" | ||
| return f"{self.__class__.__name__}()" | ||
|
|
||
|
|
||
| # noinspection PyProtectedMember | ||
| class EventWithoutExtraAttributes(Event, ABC): |
There was a problem hiding this comment.
I find this name is confusing and likely to form a grab bag. Maybe something along the lines of AlwaysEqualEventMixin?
There was a problem hiding this comment.
Mhm, it's not always equal. It's only equal if it has the same type 🤔 In my opinion it'd actually be ok if it ends up as grab bag, e.g. I see that most of this classes have as_story_string implemented with return self.type_name
There was a problem hiding this comment.
actually I like your name suggestion, will go with it 👍🏻
There was a problem hiding this comment.
I do see your point though! If we add more functionality to it we can change the name again to fit.
There was a problem hiding this comment.
Yep 👍🏻 Let's keep it narrow for now. We can't fix all problems in the e2e branch anyway
| @@ -314,21 +317,30 @@ def apply_to(self, tracker: "DialogueStateTracker") -> None: | |||
|
|
|||
| def __eq__(self, other: Any) -> bool: | |||
There was a problem hiding this comment.
We can also make this an abstractmethod now right?
There was a problem hiding this comment.
actually didn't know about this decorator. Added it 👍🏻
Proposed changes:
__eq__for Events__eq__implementationEventclass as abstractStatus (please check what you already did):
black(please check Readme for instructions)