Re: [Zope3-dev] two small bugs in zc.relationship
You are right, I should have provided tests for the changes right away. As pointed out by Stefan, I modified the `findRelationships` and `isLinked` methods in the `shared` module to no longer use logical operations to check if a `target` object was provided. As far as I can see, that makes any additional tests superfluous. What I did add at the end of container.txt are some tests for the convenience classes for one-to-one, many-to-one and one-to-many relationships. On writing the tests I discoverd another typo for the `ITargetRelationship` interface. I also changed the base interfaces for `ISourceRelationship` and `ITargetRelationship` from `IRelationship` to `IMutableRelationship`, because that's what they are. Please find the patches appended. Thanks a lot for your effort, `zc.relationship` is a great package. Replacing our own implementation of relationships by it made our code simpler and probably more stable. Markus Kemmerling relationship-patch.tgz Description: Binary data Am 02.07.2007 um 21:25 schrieb Gary Poster: On Jul 2, 2007, at 6:19 PM, Gary Poster wrote: On Jul 2, 2007, at 3:24 PM, Markus Kemmerling wrote: Hi, I discovered two smalll bugs in `zc.relationship`. Thanks, I'll get those in 24 hours or so. FWIW, generally, tests would be appreciated too, particularly of the index. I'll make one for the index: I use that heavily and the tests are pretty thorough (never thorough enough, of course). The container stuff is sadly not very well tested, and I don't have the motivation for adding to it. I just looked at the bugs and they are both in the container wrappers, as opposed to the core index. Could you whip up a diff for container.txt to test this? I'll then commit it to the trunk (slated for 2.0 RSN), make a 1.1 branch from the 1.1a tag and commit it on the branch, and make a 1.1 release and a 2.0a release. Thanks again Gary ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] two small bugs in zc.relationship
On Jul 4, 2007, at 5:59 PM, Markus Kemmerling wrote: You are right, I should have provided tests for the changes right away. Thank you very much! The following releases have your fix. http://cheeseshop.python.org/pypi/zc.relationship/1.0.2 (stable, used for ZODB 3.7/Zope 3.3/Zope 2.10) http://cheeseshop.python.org/pypi/zc.relationship/1.1 (stable, used for ZODB 3.8/Zope 3.4/Zope 2.11) http://cheeseshop.python.org/pypi/zc.relationship/2.0dev (development, used for ZODB 3.8/Zope 3.4/Zope 2.11) Gary ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
[Zope3-dev] two small bugs in zc.relationship
Hi,
I discovered two smalll bugs in `zc.relationship`. The first is just
a typo in the `ManyToOneRelationship` convenience class of the
`shared`module:
@@ -149,7 +149,7 @@
interface.implements(interfaces.IManyToOneRelationship)
def __init__(self, sources, target):
-super(OneToManyRelationship, self).__init__(sources, (target,))
+super(ManyToOneRelationShip, self).__init__((sources,),
target) @apply
def sources():
The second is a little bit more subtle. The `findRelationships`
method misbehaves if both, `source` and `target`, are not None, but
`bool(target)` evaluates to False. The fix is equally easy (and
similar to the `isLinked` method):
@@ -292,7 +292,7 @@
res = self.relationIndex.findRelationshipTokenChains(
tokenize({'source': source}),
maxDepth, filter and ResolvingFilter(filter, self),
-target and tokenize({'target': target}),
+target is not None and tokenize({'target': target})
or None,
targetFilter=minDepthFilter(minDepth))
return self._forward(res)
elif target is not None:
I discovered the last one when relating to a `target` that is a full
blown Zope object but also happens to be a yet empty container. It
certainly makes sense that an empty dictionary is considered to be
`False` when converted to a boolean. But I wouldn't have expected a
non built-in type, that *also* supports the mapping API, to be
considered as `False` as long as the mapping is empty ...
Markus Kemmerling
___
Zope3-dev mailing list
Zope3-dev@zope.org
Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] two small bugs in zc.relationship
Markus Kemmerling wrote: But I wouldn't have expected a non built-in type, that *also* supports the mapping API, to be considered as `False` as long as the mapping is empty ... Built-in or otherwise, empty containers are almost universally false in Python. -- Benji York Senior Software Engineer Zope Corporation ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] two small bugs in zc.relationship
As a general rule you should avoid taking the boolean value of objects. You never know what the result will be and how expensive the underlying operations are (e.g. computing the length of a BTree can be *very* expensive). Stefan On 2. Jul 2007, at 14:24, Markus Kemmerling wrote: But I wouldn't have expected a non built-in type, that *also* supports the mapping API, to be considered as `False` as long as the mapping is empty ... -- Anything that happens, happens. --Douglas Adams ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] two small bugs in zc.relationship
Certainly true. I just wanted the bug fix to stay in sync with the package author's coding style. He already used logical operator magic to mimic the '?:' ternary operator in a similar method. I myself prefer boring if ... else constructs. Markus Kemmerling Am 02.07.2007 um 14:51 schrieb Stefan H. Holek: As a general rule you should avoid taking the boolean value of objects. You never know what the result will be and how expensive the underlying operations are (e.g. computing the length of a BTree can be *very* expensive). Stefan On 2. Jul 2007, at 14:24, Markus Kemmerling wrote: But I wouldn't have expected a non built-in type, that *also* supports the mapping API, to be considered as `False` as long as the mapping is empty ... -- Anything that happens, happens. --Douglas Adams ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] two small bugs in zc.relationship
On Jul 2, 2007, at 3:24 PM, Markus Kemmerling wrote: Hi, I discovered two smalll bugs in `zc.relationship`. Thanks, I'll get those in 24 hours or so. FWIW, generally, tests would be appreciated too, particularly of the index. I'll make one for the index: I use that heavily and the tests are pretty thorough (never thorough enough, of course). The container stuff is sadly not very well tested, and I don't have the motivation for adding to it. Gary ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] two small bugs in zc.relationship
On Jul 2, 2007, at 6:19 PM, Gary Poster wrote: On Jul 2, 2007, at 3:24 PM, Markus Kemmerling wrote: Hi, I discovered two smalll bugs in `zc.relationship`. Thanks, I'll get those in 24 hours or so. FWIW, generally, tests would be appreciated too, particularly of the index. I'll make one for the index: I use that heavily and the tests are pretty thorough (never thorough enough, of course). The container stuff is sadly not very well tested, and I don't have the motivation for adding to it. I just looked at the bugs and they are both in the container wrappers, as opposed to the core index. Could you whip up a diff for container.txt to test this? I'll then commit it to the trunk (slated for 2.0 RSN), make a 1.1 branch from the 1.1a tag and commit it on the branch, and make a 1.1 release and a 2.0a release. Thanks again Gary ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
