Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Wed, Oct 27, 2010 at 8:56 PM, Jim Fulton j...@zope.com wrote: On Wed, Oct 27, 2010 at 1:59 PM, Hanno Schlichting ha...@hannosch.eu wrote: I suppose that depends on the application. Was the use of None intentional, or the result of sloppy coding? I'd have to check the code. I expect it to be sloppy coding. I'll make a 3.10.1 release without it and a 3.10.2a1 release with it. Thanks a lot for this! I do think these warnings are beneficial, possibly wildly so. :) As I said earlier, in 3.11, it will be an error to use an object with default comparison as a key, but loading state with such objects will only warn. I agree with them being useful. And once I'm not busy at the conference, I'll give it more proper testing. Thanks, Hanno ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Mon, Oct 25, 2010 at 11:51 PM, Jim Fulton j...@zope.com wrote: I'm inclined to treat the use of the comparison operator inherited from object in BTrees to be a bug. I plan to fix this on the trunk. Did you mean to throw warnings for simple built-in types? I'm now getting warnings for simple strings and ints, I'd expect tuples as well. All of these do inherit from object and use the default __cmp__. But their hash implementation used in the default __cmp__ should be safe to use. Hanno ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Wed, Oct 27, 2010 at 9:17 AM, Hanno Schlichting ha...@hannosch.eu wrote: On Mon, Oct 25, 2010 at 11:51 PM, Jim Fulton j...@zope.com wrote: I'm inclined to treat the use of the comparison operator inherited from object in BTrees to be a bug. I plan to fix this on the trunk. Did you mean to throw warnings for simple built-in types? No. I'm now getting warnings for simple strings and ints, I'd expect tuples as well. That's odd. I'm not. What platform and Python version? Can you give some examples? All of these do inherit from object and use the default __cmp__. I'm not sure what you mean by these, but strings, int, and tuples certainly don't use the default comparison. If they did, sorting on them would be useless. But their hash implementation used in the default __cmp__ should be safe to use. Strings, ints and tuples don't use hash for comparison. Jim -- Jim Fulton ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Wed, Oct 27, 2010 at 6:45 PM, Jim Fulton j...@zope.com wrote: If not, I wonder if the existing indexes have some bad values in them that are triggering this somehow. The relevant check is being done when loading state. I bet you have some bad keys (e.g. None) in your data structures. Could you check that? If this is what's happening, then I think the warning is useful. For 3.11 though (aka trunk) I'll rearrange things so the check isn't performed when loading state. Aha. I haven't looked into all the cases, but I did find a None value in the two examples I checked. So in an OOTree with string keys, None is considered invalid and would need to be an empty string? Thanks! Hanno ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Wed, Oct 27, 2010 at 1:59 PM, Hanno Schlichting ha...@hannosch.eu wrote: On Wed, Oct 27, 2010 at 6:45 PM, Jim Fulton j...@zope.com wrote: If not, I wonder if the existing indexes have some bad values in them that are triggering this somehow. The relevant check is being done when loading state. I bet you have some bad keys (e.g. None) in your data structures. Could you check that? If this is what's happening, then I think the warning is useful. For 3.11 though (aka trunk) I'll rearrange things so the check isn't performed when loading state. Aha. I haven't looked into all the cases, but I did find a None value in the two examples I checked. So in an OOTree with string keys, None is considered invalid Short answer: yes None's ordering wrt strings can change, and I believe it has in the past. It compares solely on address. In Python 3, it is an error to compare None with something else. Using None as a BTree key seems like a very bad idea. and would need to be an empty string? I suppose that depends on the application. Was the use of None intentional, or the result of sloppy coding? I'm thinking that this change is scary enough that I'm going to separate it from 3.10.1, especially since 3.10.1 has some other bug fixes that are pretty important. I'll make a 3.10.1 release without it and a 3.10.2a1 release with it. I do think these warnings are beneficial, possibly wildly so. :) As I said earlier, in 3.11, it will be an error to use an object with default comparison as a key, but loading state with such objects will only warn. Thanks much for trying this out! Jim -- Jim Fulton ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Mon, Oct 25, 2010 at 5:51 PM, Jim Fulton j...@zope.com wrote: I'm inclined to treat the use of the comparison operator inherited from object in BTrees to be a bug. I plan to fix this on the trunk. I'm tempted to fix this in 10.1. This change would make it impossible to add keys to BTrees or buckets or to add items to BTree-based sets if the key or items inherits it's comparison from object. This would only apply to instances of new-style classes, including persistent objects. (It wouldn't affect old-style-class instances, which are too hard to introspect.) Of course, it also wouldn't protect anybody from objects with broken comparisons or objects that base reasonably sane comparison on comparison of sub objects with bad comparison, such as: (object(), ) :) I do think it would catch a common class of bug involving using objects as keys. Jim -- Jim Fulton ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
Or perhaps make it emit DeprecationWarnings, but continue working. Then make it a fatal error in the next minor/major release. +1 Pedro -- José Pedro Ferreira Indico Team IT-UDS-AVC 513-R-0042 CERN, Geneva, Switzerland ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On 25/10/2010 23:34, Marius Gedminas wrote: Or perhaps make it emit DeprecationWarnings, but continue working. Then make it a fatal error in the next minor/major release. Well, not a DeprecationWarning... Is there a DataLossWarning? Still, +1 on the warning followed by exception in 2 releases... Chris -- Simplistix - Content Management, Batch Processing Python Consulting - http://www.simplistix.co.uk ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Mon, Oct 25, 2010 at 6:34 PM, Marius Gedminas mar...@gedmin.as wrote: ... Or perhaps make it emit DeprecationWarnings, but continue working. Then make it a fatal error in the next minor/major release. I like this idea. I think I'm going to use UserWarning because it isn't disabled in Python 2.7 afaik. Otherwise, I would liie deprecation warning because it suggest that things are bad and going to get worse. :) Jim -- Jim Fulton ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
[ZODB-Dev] Default comparison considered harmful in BTrees.
I'm inclined to treat the use of the comparison operator inherited from object in BTrees to be a bug. I plan to fix this on the trunk. I'm tempted to fix this in 10.1. This change would make it impossible to add keys to BTrees or buckets or to add items to BTree-based sets if the key or items inherits it's comparison from object. This would only apply to instances of new-style classes, including persistent objects. (It wouldn't affect old-style-class instances, which are too hard to introspect.) Thoughts? Jim -- Jim Fulton ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On 10/25/10 10:51 PM, Jim Fulton wrote: I'm inclined to treat the use of the comparison operator inherited from object in BTrees to be a bug. I plan to fix this on the trunk. I'm tempted to fix this in 10.1. This change would make it impossible to add keys to BTrees or buckets or to add items to BTree-based sets if the key or items inherits it's comparison from object. This would only apply to instances of new-style classes, including persistent objects. (It wouldn't affect old-style-class instances, which are too hard to introspect.) Thoughts? The motivation here is that the comparison inherited from object compares the objects' memory locations, which is not stable beyond deactivation for persistent objects, and therefore not suitable for use as a BTree key. Correct? Preventing people from making that mistake sounds like a good thing to me. -- David Glick Web Developer davidgl...@groundwire.org 206.286.1235x32 Groundwire: You Are Connected http://groundwire.org Online tools and stratgies for the environmental movement. Sign up for Groundwire News! http://groundwire.org/email-capture ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Mon, Oct 25, 2010 at 5:58 PM, David Glick davidgl...@groundwire.org wrote: On 10/25/10 10:51 PM, Jim Fulton wrote: I'm inclined to treat the use of the comparison operator inherited from object in BTrees to be a bug. I plan to fix this on the trunk. I'm tempted to fix this in 10.1. This change would make it impossible to add keys to BTrees or buckets or to add items to BTree-based sets if the key or items inherits it's comparison from object. This would only apply to instances of new-style classes, including persistent objects. (It wouldn't affect old-style-class instances, which are too hard to introspect.) Thoughts? The motivation here is that the comparison inherited from object compares the objects' memory locations, which is not stable beyond deactivation for persistent objects, and therefore not suitable for use as a BTree key. Correct? Yup. Thanks for clarifying. Preventing people from making that mistake sounds like a good thing to me. It will likely reveal that current applications are broken. This will likely cause some pain. Where previously, apps simply lost data, now they'll error. I'm afraid that this might be too disruptive for a bug-fix release. Jim -- Jim Fulton ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On 10/25/10 11:07 PM, Jim Fulton wrote: On Mon, Oct 25, 2010 at 5:58 PM, David Glick davidgl...@groundwire.org wrote: On 10/25/10 10:51 PM, Jim Fulton wrote: I'm inclined to treat the use of the comparison operator inherited from object in BTrees to be a bug. I plan to fix this on the trunk. I'm tempted to fix this in 10.1. This change would make it impossible to add keys to BTrees or buckets or to add items to BTree-based sets if the key or items inherits it's comparison from object. This would only apply to instances of new-style classes, including persistent objects. (It wouldn't affect old-style-class instances, which are too hard to introspect.) Thoughts? The motivation here is that the comparison inherited from object compares the objects' memory locations, which is not stable beyond deactivation for persistent objects, and therefore not suitable for use as a BTree key. Correct? Yup. Thanks for clarifying. Preventing people from making that mistake sounds like a good thing to me. It will likely reveal that current applications are broken. This will likely cause some pain. Where previously, apps simply lost data, now they'll error. I'm afraid that this might be too disruptive for a bug-fix release. Maybe add it as an option defaulted to off? So that app developers who want to check whether they have this problem can easily do so, and have the extra protection once they've fixed any issues? -- David Glick Web Developer davidgl...@groundwire.org 206.286.1235x32 Groundwire: You Are Connected http://groundwire.org Online tools and stratgies for the environmental movement. Sign up for Groundwire News! http://groundwire.org/email-capture ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Mon, Oct 25, 2010 at 11:14:21PM +0100, David Glick wrote: On 10/25/10 11:07 PM, Jim Fulton wrote: On Mon, Oct 25, 2010 at 5:58 PM, David Glick davidgl...@groundwire.org wrote: On 10/25/10 10:51 PM, Jim Fulton wrote: I'm inclined to treat the use of the comparison operator inherited from object in BTrees to be a bug. I plan to fix this on the trunk. I'm tempted to fix this in 10.1. This change would make it impossible to add keys to BTrees or buckets or to add items to BTree-based sets if the key or items inherits it's comparison from object. This would only apply to instances of new-style classes, including persistent objects. (It wouldn't affect old-style-class instances, which are too hard to introspect.) Thoughts? I like this. The motivation here is that the comparison inherited from object compares the objects' memory locations, which is not stable beyond deactivation for persistent objects, and therefore not suitable for use as a BTree key. Correct? Yup. Thanks for clarifying. Preventing people from making that mistake sounds like a good thing to me. It will likely reveal that current applications are broken. This will likely cause some pain. Where previously, apps simply lost data, now they'll error. I'm afraid that this might be too disruptive for a bug-fix release. Maybe add it as an option defaulted to off? So that app developers who want to check whether they have this problem can easily do so, and have the extra protection once they've fixed any issues? Or perhaps make it emit DeprecationWarnings, but continue working. Then make it a fatal error in the next minor/major release. Marius Gedminas -- Include me out. signature.asc Description: Digital signature ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
