Skip to content
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

implement hash and __eq__ for vDDDTypes #492

Merged
merged 6 commits into from Nov 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions CHANGES.rst
Expand Up @@ -14,11 +14,11 @@ Breaking changes:

New features:

- ...
- vDDDTypes is hashable #487 #492 [niccokunzmann]

Bug fixes:

- ...
- vDDDTypes' equality also checks the dt attribute #497 #492 [niccokunzmann]

5.0.2 (2022-11-03)
------------------
Expand Down
5 changes: 4 additions & 1 deletion src/icalendar/prop.py
Expand Up @@ -320,9 +320,12 @@ def to_ical(self):

def __eq__(self, other):
if isinstance(other, vDDDTypes):
return self.params == other.params
return self.params == other.params and self.dt == other.dt
jacadzaca marked this conversation as resolved.
Show resolved Hide resolved
return False

def __hash__(self):
return hash(self.dt)

@classmethod
def from_ical(cls, ical, timezone=None):
if isinstance(ical, cls):
Expand Down
60 changes: 37 additions & 23 deletions src/icalendar/tests/test_unit_prop.py
Expand Up @@ -4,10 +4,12 @@
from datetime import timedelta
from icalendar.parser import Parameters
import unittest
from icalendar.prop import vDatetime
from icalendar.prop import vDatetime, vDDDTypes
from icalendar.windows_to_olson import WINDOWS_TO_OLSON

import pytest
import pytz
from copy import deepcopy
from dateutil import tz


class TestProp(unittest.TestCase):
Expand Down Expand Up @@ -112,27 +114,6 @@ def test_prop_vDDDTypes(self):
# Bad input
self.assertRaises(ValueError, vDDDTypes, 42)

def test_vDDDTypes_equivalance(self):
from ..prop import vDDDTypes
from copy import deepcopy

vDDDTypes = [
vDDDTypes(pytz.timezone('US/Eastern').localize(datetime(year=2022, month=7, day=22, hour=12, minute=7))),
vDDDTypes(datetime(year=2022, month=7, day=22, hour=12, minute=7)),
vDDDTypes(date(year=2022, month=7, day=22)),
vDDDTypes(time(hour=22, minute=7, second=2))]

for v_type in vDDDTypes:
self.assertEqual(v_type, deepcopy(v_type))
for other in vDDDTypes:
if other is v_type:
continue
self.assertNotEqual(v_type, other)

# see if equivalnce fails for other types
self.assertNotEqual(v_type, 42)
self.assertNotEqual(v_type, 'test')

def test_prop_vDate(self):
from ..prop import vDate

Expand Down Expand Up @@ -518,6 +499,39 @@ def test_prop_TypesFactory(self):
)



vDDDTypes_list = [
vDDDTypes(pytz.timezone('US/Eastern').localize(datetime(year=2022, month=7, day=22, hour=12, minute=7))),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this use the in_timezone fixture from the conftest.py?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to add not too much. Hm. Since many of the arguments do not use a tz, that would create redundant tests.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain how would they be redundant? I don't quite get that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A second or third case with timezone should do the job too. One with same timezone and one with different time zone but same instant in time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ae9ac71 I added another time zone. I think, this covers the case:

  • time zone differs
  • time zone different from no time zone

Since no time zone is also a time zone in my view, all the cases would be covered. What do you think?

vDDDTypes(datetime(year=2022, month=7, day=22, hour=12, minute=7)),
vDDDTypes(datetime(year=2022, month=7, day=22, hour=12, minute=7, tzinfo=tz.UTC)),
vDDDTypes(date(year=2022, month=7, day=22)),
vDDDTypes(date(year=2022, month=7, day=23)),
vDDDTypes(time(hour=22, minute=7, second=2))
]

def identity(x):
return x

@pytest.mark.parametrize("map", [
deepcopy,
identity,
hash,
])
@pytest.mark.parametrize("v_type", vDDDTypes_list)
@pytest.mark.parametrize("other", vDDDTypes_list)
def test_vDDDTypes_equivalance(map, v_type, other):
if v_type is other:
assert map(v_type) == map(other), f"identity implies equality: {map.__name__}()"
assert not (map(v_type) != map(other)), f"identity implies equality: {map.__name__}()"
jacadzaca marked this conversation as resolved.
Show resolved Hide resolved
else:
assert map(v_type) != map(other), f"expected inequality: {map.__name__}()"
assert not (map(v_type) == map(other)), f"expected inequality: {map.__name__}()"
jacadzaca marked this conversation as resolved.
Show resolved Hide resolved

@pytest.mark.parametrize("v_type", vDDDTypes_list)
def test_inequality_with_different_types(v_type):
assert v_type != 42
assert v_type != 'test'

class TestPropertyValues(unittest.TestCase):

def test_vDDDLists_timezone(self):
Expand Down