InterfaceClass __cmp method obtains module name incorrectly

[David Glick]

InterfaceClass provides custom __gt__ and __lt__ methods based on a method called __cmp, which tries to compare a (interface name, module name) tuple for the two given interfaces. However, this code tries to retrieve the module name via getattr(iface.__module__, '__name__', ''), when iface.__module__ is already the module name. So the second element of the tuple is always an empty string, and two interfaces that have the same name but come from different modules will not compare properly. e.g. the following weird behavior:

from x import IContentish
from y import IContentish as IContentish2
>>> IContentish < IContentish2
False
>>> IContentish > IContentish2
False
>>> IContentish == IContentish2
False

This has ugly implications when interfaces are used as BTree keys, as the ordering is not trichotomous.

I am attaching a patch which I believe fixes the issue.

[Tres Seaver (tseaver)]

The original code is clearly wrong: it is written on the assumption that an interface's '__module__'' attribute is a module object, rather than its dotted name.

[Tres Seaver (tseaver)]

Not only that, but the cutesy subtraction at the end of the method is unclear and completely undocumented. I woul dbe in favor of completely rewriting this helper function to avoid the cute factor, with vastly better tests.

[David Glick (davisagli)]

The subtraction was added in Lennart's Python 3 compatibility branch (c.f. http://zope3.pov.lt/trac/changeset/110699), replacing the cmp that had been there before. According to http://docs.python.org/release/3.0.1/whatsnew/3.0.html#ordering-comparisons, the subtraction is the recommended replacement for cmp in Python 3.

I can go ahead and merge my patch, unless someone objects. Tres, did you have some specific test in mind that is missing?

[Tres Seaver (tseaver)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

David Glick wrote:
> The subtraction was added in Lennart's Python 3 compatibility branch
> (c.f. http://zope3.pov.lt/trac/changeset/110699), replacing the cmp that
> had been there before. According to
> http://docs.python.org/release/3.0.1/whatsnew/3.0.html#ordering-
> comparisons, the subtraction is the recommended replacement for cmp in
> Python 3.
>
> I can go ahead and merge my patch, unless someone objects. Tres, did you
> have some specific test in mind that is missing?

I would like to see explicit tests for all six comparison operators,
including for the edge cases (comparison with None in both directions,
comparison with the identical object, etc.)

Attaching a patch which adds those two edge case tests. Your test was
ok for demonstrating the failure, but I would like to see it assert the
other operations as well.

Tres.
- --
===================================================================
Tres Seaver [phone] <email address hidden>
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkvYeZsACgkQ+gerLs4ltQ5B7wCeLmljMGXCOvVTNiaLJ33M0YVd
i/EAnion65qL5zheWJ7y9fblYZMEYuul
=NH1S
-----END PGP SIGNATURE-----

[Gaudenz Steinlin (gaudenz-debian)]

Is there a reason why this patch hasn't been applied so far?

My zc.relation ICatalog relation catalog gets corrupted from time to time and this might very well be caused by this bug.

See also: http://sourceforge.net/mailarchive/message.php?msg_id=27474673

I'm a bit reluctant to apply this patch to a production system because I'm not sure if there might be bad side effects. But I could not trigger the corruption bug in my development environment. The patch does not seem to do any harm in the development environment.

[Tres Seaver (tseaver)]

Thanks for the reminder. The patched version is now released as zope.interface 3.6.3:

  http://pypi.python.org/pypi/zope.interface/3.6.3