Skip to content

Commit

Permalink
Include a field's interface in equality and hashing.
Browse files Browse the repository at this point in the history
Fixes #40
  • Loading branch information
jamadden committed Aug 31, 2018
1 parent b662565 commit da933db
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 76 deletions.
10 changes: 10 additions & 0 deletions CHANGES.rst
Expand Up @@ -30,6 +30,16 @@
compatibility problem. See `issue 36
<https://github.com/zopefoundation/zope.schema/issues/36>`_.

- ``Field`` instances are only equal when their ``.interface`` is
equal. In practice, this means that two otherwise identical fields
of separate schemas are not equal, do not hash the same, and can
both be members of the same ``dict`` or ``set``. Prior to this
release, when hashing was identity based but only worked on Python
2, that was the typical behaviour. (Field objects that are *not*
members of a schema continue to compare and hash equal if they have
the same attributes and interfaces.) See `issue 40
<https://github.com/zopefoundation/zope.schema/issues/40>`_.

- Orderable fields, including ``Int``, ``Float``, ``Decimal``,
``Timedelta``, ``Date`` and ``Time``, can now have a
``missing_value`` without needing to specify concrete ``min`` and
Expand Down
15 changes: 9 additions & 6 deletions src/zope/schema/_bootstrapfields.py
Expand Up @@ -119,7 +119,7 @@ class Field(Attribute):
default = DefaultProperty('default')

# These were declared as slots in zope.interface, we override them here to
# get rid of the dedcriptors so they don't break .bind()
# get rid of the descriptors so they don't break .bind()
__name__ = None
interface = None
_Element__tagged_values = None
Expand Down Expand Up @@ -206,21 +206,24 @@ def __get_property_names_to_compare(self):
names.update(getFields(interface))

# order will be different always, don't compare it
if 'order' in names:
del names['order']
names.pop('order', None)
return names

def __hash__(self):
# Equal objects should have equal hashes;
# equal hashes does not imply equal objects.
value = (type(self),) + tuple(self.__get_property_names_to_compare())
value = (type(self), self.interface) + tuple(self.__get_property_names_to_compare())
return hash(value)

def __eq__(self, other):
# should be the same type
if type(self) != type(other):
# should be the same type and in the same interface (or no interface at all)
if self is other:
return True

if type(self) != type(other) or self.interface != other.interface:
return False


# should have the same properties
names = self.__get_property_names_to_compare()
# XXX: What about the property names of the other object? Even
Expand Down
162 changes: 92 additions & 70 deletions src/zope/schema/tests/test__bootstrapfields.py
Expand Up @@ -142,7 +142,88 @@ def _factory(context):
self.assertEqual(_called_with, [inst.context])


class FieldTests(unittest.TestCase):
class EqualityTestsMixin(object):

def _getTargetClass(self):
raise NotImplementedError

def _makeOne(self, *args, **kw):
return self._getTargetClass()(*args, **kw)

def test_is_hashable(self):
field = self._makeOne()
hash(field) # doesn't raise

def test_equal_instances_have_same_hash(self):
# Equal objects should have equal hashes
field1 = self._makeOne()
field2 = self._makeOne()
self.assertIsNot(field1, field2)
self.assertEqual(field1, field2)
self.assertEqual(hash(field1), hash(field2))

def test_instances_in_different_interfaces_not_equal(self):
from zope import interface

field1 = self._makeOne()
field2 = self._makeOne()
self.assertEqual(field1, field2)
self.assertEqual(hash(field1), hash(field2))

class IOne(interface.Interface):
one = field1

class ITwo(interface.Interface):
two = field2

self.assertEqual(field1, field1)
self.assertEqual(field2, field2)
self.assertNotEqual(field1, field2)
self.assertNotEqual(hash(field1), hash(field2))

def test_hash_across_unequal_instances(self):
# Hash equality does not imply equal objects.
# Our implementation only considers property names,
# not values. That's OK, a dict still does the right thing.
field1 = self._makeOne(title=u'foo')
field2 = self._makeOne(title=u'bar')
self.assertIsNot(field1, field2)
self.assertNotEqual(field1, field2)
self.assertEqual(hash(field1), hash(field2))

d = {field1: 42}
self.assertIn(field1, d)
self.assertEqual(42, d[field1])
self.assertNotIn(field2, d)
with self.assertRaises(KeyError):
d.__getitem__(field2)

def test___eq___different_type(self):
left = self._makeOne()

class Derived(self._getTargetClass()):
pass
right = Derived()
self.assertNotEqual(left, right)
self.assertTrue(left != right)

def test___eq___same_type_different_attrs(self):
left = self._makeOne(required=True)
right = self._makeOne(required=False)
self.assertNotEqual(left, right)
self.assertTrue(left != right)

def test___eq___same_type_same_attrs(self):
left = self._makeOne()
self.assertEqual(left, left)

right = self._makeOne()
self.assertEqual(left, right)
self.assertFalse(left != right)


class FieldTests(EqualityTestsMixin,
unittest.TestCase):

def _getTargetClass(self):
from zope.schema._bootstrapfields import Field
Expand Down Expand Up @@ -304,27 +385,6 @@ def _fail(value):
field._type = int
field.validate(1) # doesn't raise

def test___eq___different_type(self):
left = self._makeOne()

class Derived(self._getTargetClass()):
pass
right = Derived()
self.assertEqual(left == right, False)
self.assertEqual(left != right, True)

def test___eq___same_type_different_attrs(self):
left = self._makeOne(required=True)
right = self._makeOne(required=False)
self.assertEqual(left == right, False)
self.assertEqual(left != right, True)

def test___eq___same_type_same_attrs(self):
left = self._makeOne()
right = self._makeOne()
self.assertEqual(left == right, True)
self.assertEqual(left != right, False)

def test_get_miss(self):
field = self._makeOne(__name__='nonesuch')
inst = DummyInst()
Expand Down Expand Up @@ -364,44 +424,14 @@ def test_set_hit(self):
field.set(inst, 'AFTER')
self.assertEqual(inst.extant, 'AFTER')

def test_is_hashable(self):
field = self._makeOne()
hash(field) # doesn't raise

def test_equal_instances_have_same_hash(self):
# Equal objects should have equal hashes
field1 = self._makeOne()
field2 = self._makeOne()
self.assertIsNot(field1, field2)
self.assertEqual(field1, field2)
self.assertEqual(hash(field1), hash(field2))

def test_hash_across_unequal_instances(self):
# Hash equality does not imply equal objects.
# Our implementation only considers property names,
# not values. That's OK, a dict still does the right thing.
field1 = self._makeOne(title=u'foo')
field2 = self._makeOne(title=u'bar')
self.assertIsNot(field1, field2)
self.assertNotEqual(field1, field2)
self.assertEqual(hash(field1), hash(field2))

d = {field1: 42}
self.assertIn(field1, d)
self.assertEqual(42, d[field1])
self.assertNotIn(field2, d)
with self.assertRaises(KeyError):
d[field2]

class ContainerTests(unittest.TestCase):
class ContainerTests(EqualityTestsMixin,
unittest.TestCase):

def _getTargetClass(self):
from zope.schema._bootstrapfields import Container
return Container

def _makeOne(self, *args, **kw):
return self._getTargetClass()(*args, **kw)

def test_validate_not_required(self):
field = self._makeOne(required=False)
field.validate(None)
Expand Down Expand Up @@ -529,15 +559,13 @@ def test_validate_too_long(self):
self.assertRaises(TooLong, mml._validate, (0, 1, 2))


class TextTests(unittest.TestCase):
class TextTests(EqualityTestsMixin,
unittest.TestCase):

def _getTargetClass(self):
from zope.schema._bootstrapfields import Text
return Text

def _makeOne(self, *args, **kw):
return self._getTargetClass()(*args, **kw)

def test_ctor_defaults(self):
from zope.schema._compat import text_type
txt = self._makeOne()
Expand Down Expand Up @@ -593,15 +621,13 @@ def test_fromUnicode_hit(self):
self.assertEqual(txt.fromUnicode(deadbeef), deadbeef)


class TextLineTests(unittest.TestCase):
class TextLineTests(EqualityTestsMixin,
unittest.TestCase):

def _getTargetClass(self):
from zope.schema._field import TextLine
return TextLine

def _makeOne(self, *args, **kw):
return self._getTargetClass()(*args, **kw)

def test_class_conforms_to_ITextLine(self):
from zope.interface.verify import verifyClass
from zope.schema.interfaces import ITextLine
Expand Down Expand Up @@ -711,15 +737,13 @@ def test_constraint(self):
self.assertEqual(field.constraint(u'abc\ndef'), False)


class BoolTests(unittest.TestCase):
class BoolTests(EqualityTestsMixin,
unittest.TestCase):

def _getTargetClass(self):
from zope.schema._bootstrapfields import Bool
return Bool

def _makeOne(self, *args, **kw):
return self._getTargetClass()(*args, **kw)

def test_ctor_defaults(self):
txt = self._makeOne()
self.assertEqual(txt._type, bool)
Expand Down Expand Up @@ -774,16 +798,14 @@ def test_missing_value_no_min_or_max(self):
self.assertEqual(self.mvm_missing_value, field.missing_value)


class IntTests(OrderableMissingValueMixin,
class IntTests(EqualityTestsMixin,
OrderableMissingValueMixin,
unittest.TestCase):

def _getTargetClass(self):
from zope.schema._bootstrapfields import Int
return Int

def _makeOne(self, *args, **kw):
return self._getTargetClass()(*args, **kw)

def test_ctor_defaults(self):
from zope.schema._compat import integer_types
txt = self._makeOne()
Expand Down

0 comments on commit da933db

Please sign in to comment.