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

Should Field objects include their owning interface in equality/hashing? #40

Closed
jamadden opened this issue Aug 13, 2018 · 5 comments · Fixed by #51
Closed

Should Field objects include their owning interface in equality/hashing? #40

jamadden opened this issue Aug 13, 2018 · 5 comments · Fixed by #51
Assignees

Comments

@jamadden
Copy link
Member

Currently they don't include that, leaving open the possibility that two different Field objects from two different interfaces are equal, which may or may not be the intent.

>>> from zope.interface import Interface
>>> from zope.schema import Int
>>> class IOne(Interface):
...    one = Int(title=u'title')
...
>>> class ITwo(Interface):
...    two = Int(title=u'title')
...
>>> IOne['one'] == ITwo['two']
True
@mgedmin
Copy link
Member

mgedmin commented Aug 13, 2018

I haven't given it any deep thoughts, but the current behavior seems reasonable to me.

@jamadden
Copy link
Member Author

I'll clarify that the possible motivation for this change comes from this comment, based on the old behaviour of hash(field) that only accounted for object identity, not equality:

code that uses fields as dict keys may rely on mixing fields from different schemas that are not identical but compare as equal otherwise.

@icemac
Copy link
Member

icemac commented Aug 27, 2018

I think we should stick as close as possible to the old behaviour (pre Python 3) of zope.schema as this will not break any code which is out there. Yes, this might hamper or even prevent other new uses, but I would value the existing code more than the one that might be written in future.

@jamadden
Copy link
Member Author

One way of looking at it is that including the interface in hashing is closer to what zope.schema used to do: hashing was identity based, so hash(IOne['one']) != hash(ITwo['two']) (and hashing only worked on Python 2, so anyone relying on hashing could not port to Python 3). But now that hashing works on Python 3, and is consistent with equality, hash(IOne['one']) == hash(ITwo['two']). Previously on Python 2, a dict could include both IOne['one'] and ITwo['two']:

>>> d = {}
>>> d[IOne['one']] = 'one'
>>> d[ITwo['two']] = 'two'
>>> d
{<zope.schema._bootstrapfields.Int at 0x10fe64ed0>: 'one',
 <zope.schema._bootstrapfields.Int at 0x10ffdb610>: 'two'}

Now it cannot:

>>> d = {}
>>> d[IOne['one']] = 'one'
>>> d[ITwo['two']] = 'two'
>>> d
{<zope.schema._bootstrapfields.Int at 0x10bc25d90>: 'two'}

Including the interface in equality and hashing would restore the ability to have both fields in a dict, while staying true to the language spec.

@icemac
Copy link
Member

icemac commented Aug 31, 2018

+1 to include the interface in the hash computation. The current behaviour @jamadden has shown in the previous comment seems irritating to me. I'd never expect that two fields on different interfaces with different names are the same because they have the same configuration.
Fields almost always have a context (the interface) which makes them unequal because the interface becomes part of the field. They are the same as long as they are not part of an interface. At the moment a field becomes part of the interface it is no longer the same as a field on another interface or a field which is not part of an interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants