-
-
Notifications
You must be signed in to change notification settings - Fork 171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix #526 #550
fix #526 #550
Conversation
@yassirasad and @Lx does this fix work for you folks? |
src/icalendar/tests/test_unit_cal.py
Outdated
'issue_178_custom_component_contains_other', | ||
'issue_178_custom_component_inside_other', | ||
'issue_526_calendar_with_events', | ||
'issue_526_calendar_with_different_events', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the lists are always supposed to be equal, they could be in one variable.
@@ -436,6 +436,22 @@ def __repr__(self): | |||
subs = ', '.join(str(it) for it in self.subcomponents) | |||
return f"{self.name or type(self).__name__}({dict(self)}{', ' + subs if subs else ''})" | |||
|
|||
def __eq__(self, other): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a rule, overriding eq should also override hash. But I think, as long as the equality gets reduced, equality still implies hash equality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that implementing a __hash__
for sub-class of CaselessDict is not straightforward as per [this](https://github.com/collective/icalendar/pull/391/files#r957502946}
# check for set equivalence. We have to iterate over the subcomponents | ||
# and look for each of them in the list. | ||
for subcomponent in self.subcomponents: | ||
if subcomponent not in other.subcomponents: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a subset test. a == b does not imply b == a.
…components as equal to the bigger component
I think it is perfect. |
I guess, we can create a new release. |
fix #526