Re: [Zope-dev] Request for comment on zope.schema patch for bug 969350

2012-04-02 Thread Alexandre Garel

Le 30/03/2012 22:54, Marius Gedminas a écrit :

On Fri, Mar 30, 2012 at 06:36:13PM +0200, Alexandre Garel wrote:

Hello,

I fill a bug report for zope.schema :

"zope.schema does not handle well nested object fields"
https://bugs.launchpad.net/zope.schema/+bug/969350

my patch change a particular behaviour (binding of Choice field
nested in Object field)

I explain why on the ticket.

Any comment would be appreciated.

Here's the diff attached to that bug:


Thanks for taking time.

As you can see below I'm ok for all your comment but one, which needs 
further discussion.



Index: src/zope/schema/tests/test_objectfield_nested.py
===
--- src/zope/schema/tests/test_objectfield_nested.py(révision 0)
+++ src/zope/schema/tests/test_objectfield_nested.py(révision 0)
@@ -0,0 +1,74 @@
+##
+#
+# Copyright (c) 2001, 2002 Zope Foundation and Contributors.

Probably should be 2012 ;)


Hum copy/paste killed me :-)


+# All Rights Reserved.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.1 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE.
+#
+##
+"""This test exercises Object fields in the case of nestment inside list

s/netsment/nesting/, although the sentence could use some rewriting.


ok for nesting

+to verify binding correcly happens at validation time
+"""
+
+from unittest import TestCase, TestSuite, main, makeSuite
+
+from zope.interface import Interface, implementer
+from zope.schema import Choice, Object, Set
+from zope.schema.interfaces import IContextSourceBinder, WrongContainedType
+from zope.schema.vocabulary import SimpleVocabulary, SimpleTerm
+from zope.testing.cleanup import CleanUp
+
+
+@implementer(IContextSourceBinder)
+class EnumContext(object):
+
+def __call__(self, context):
+return SimpleVocabulary([
+SimpleTerm(value=v, token=v, title='Letter %s' % v)
+for v in context])
+
+
+class IMultipleChoice(Interface):
+choices = Set(value_type=Choice(source=EnumContext()))
+
+
+@implementer(IMultipleChoice)
+class Choices(object):
+
+def __iter__(self):
+return iter(range(5))  # to have EnumSource work

EnumSource?  Do you mean EnumContext?


Yes, I'll correct


+
+def __init__(self, choices):
+self.choices = choices
+
+
+class IFavorites(Interface):
+fav = Object(title=u"Favorites number", schema=IMultipleChoice)
+
+
+class ObjectNestedTest(CleanUp, TestCase):
+"""Test the Object Field in cases of nested fields"""
+
+def test_validate_nested(self):
+context = range(5)

This is a bit confusing.  IField.bind() is usually passed a context that
is an object that has an attribute with that field.  You're passing a
list of numbers.  Would the test also work if you had context = object()?
In fact as I just wan't to test validation, I did use a range. I would 
of course work with an object which is iterable (because EnumContext 
needs it)

+# must not raise
+IFavorites['fav'].bind(context).validate(Choices(set([1, 3])))
+# checking against dictionnary do work

I don't see any dictionaries...  Perhaps you meant 'checking of set
values against the source works'?

Hum sorry, that may be a copy/paste remainder there again :-/


+self.assertRaises(
+WrongContainedType,
+IFavorites['fav'].bind(context).validate, Choices(set([1, 8])))
+
+
+def test_suite():
+suite = TestSuite()
+suite.addTest(makeSuite(ObjectNestedTest))
+return suite
+
+
+if __name__ == '__main__':
+main(defaultTest='test_suite')
Index: src/zope/schema/_field.py
===
--- src/zope/schema/_field.py   (révision 124803)
+++ src/zope/schema/_field.py   (copie de travail)
@@ -492,12 +492,7 @@
  if not IMethod.providedBy(schema[name]):
  try:
  attribute = schema[name]
-if IChoice.providedBy(attribute):
-# Choice must be bound before validation otherwise
-# IContextSourceBinder is not iterable in validation
-bound = attribute.bind(value)
-bound.validate(getattr(value, name))
-elif IField.providedBy(attribute):
+if IField.providedBy(attribute):
  # validate attributes that are fields
  attribute.validate(getattr(value, name))
  except ValidationError as error:

Re: [Zope-dev] Request for comment on zope.schema patch for bug 969350

2012-03-30 Thread Marius Gedminas
On Fri, Mar 30, 2012 at 06:36:13PM +0200, Alexandre Garel wrote:
> Hello,
> 
> I fill a bug report for zope.schema :
> 
> "zope.schema does not handle well nested object fields"
> https://bugs.launchpad.net/zope.schema/+bug/969350
> 
> my patch change a particular behaviour (binding of Choice field
> nested in Object field)
> 
> I explain why on the ticket.
> 
> Any comment would be appreciated.

Here's the diff attached to that bug:

> Index: src/zope/schema/tests/test_objectfield_nested.py
> ===
> --- src/zope/schema/tests/test_objectfield_nested.py  (révision 0)
> +++ src/zope/schema/tests/test_objectfield_nested.py  (révision 0)
> @@ -0,0 +1,74 @@
> +##
> +#
> +# Copyright (c) 2001, 2002 Zope Foundation and Contributors.

Probably should be 2012 ;)

> +# All Rights Reserved.
> +#
> +# This software is subject to the provisions of the Zope Public License,
> +# Version 2.1 (ZPL).  A copy of the ZPL should accompany this distribution.
> +# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
> +# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> +# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
> +# FOR A PARTICULAR PURPOSE.
> +#
> +##
> +"""This test exercises Object fields in the case of nestment inside list

s/netsment/nesting/, although the sentence could use some rewriting.

> +to verify binding correcly happens at validation time
> +"""
> +
> +from unittest import TestCase, TestSuite, main, makeSuite
> +
> +from zope.interface import Interface, implementer
> +from zope.schema import Choice, Object, Set
> +from zope.schema.interfaces import IContextSourceBinder, WrongContainedType
> +from zope.schema.vocabulary import SimpleVocabulary, SimpleTerm
> +from zope.testing.cleanup import CleanUp
> +
> +
> +@implementer(IContextSourceBinder)
> +class EnumContext(object):
> +
> +def __call__(self, context):
> +return SimpleVocabulary([
> +SimpleTerm(value=v, token=v, title='Letter %s' % v)
> +for v in context])
> +
> +
> +class IMultipleChoice(Interface):
> +choices = Set(value_type=Choice(source=EnumContext()))
> +
> +
> +@implementer(IMultipleChoice)
> +class Choices(object):
> +
> +def __iter__(self):
> +return iter(range(5))  # to have EnumSource work

EnumSource?  Do you mean EnumContext?

> +
> +def __init__(self, choices):
> +self.choices = choices
> +
> +
> +class IFavorites(Interface):
> +fav = Object(title=u"Favorites number", schema=IMultipleChoice)
> +
> +
> +class ObjectNestedTest(CleanUp, TestCase):
> +"""Test the Object Field in cases of nested fields"""
> +
> +def test_validate_nested(self):
> +context = range(5)

This is a bit confusing.  IField.bind() is usually passed a context that
is an object that has an attribute with that field.  You're passing a
list of numbers.  Would the test also work if you had context = object()?

> +# must not raise
> +IFavorites['fav'].bind(context).validate(Choices(set([1, 3])))
> +# checking against dictionnary do work

I don't see any dictionaries...  Perhaps you meant 'checking of set
values against the source works'?

> +self.assertRaises(
> +WrongContainedType,
> +IFavorites['fav'].bind(context).validate, Choices(set([1, 8])))
> +
> +
> +def test_suite():
> +suite = TestSuite()
> +suite.addTest(makeSuite(ObjectNestedTest))
> +return suite
> +
> +
> +if __name__ == '__main__':
> +main(defaultTest='test_suite')

> Index: src/zope/schema/_field.py
> ===
> --- src/zope/schema/_field.py (révision 124803)
> +++ src/zope/schema/_field.py (copie de travail)
> @@ -492,12 +492,7 @@
>  if not IMethod.providedBy(schema[name]):
>  try:
>  attribute = schema[name]
> -if IChoice.providedBy(attribute):
> -# Choice must be bound before validation otherwise
> -# IContextSourceBinder is not iterable in validation
> -bound = attribute.bind(value)
> -bound.validate(getattr(value, name))
> -elif IField.providedBy(attribute):
> +if IField.providedBy(attribute):
>  # validate attributes that are fields
>  attribute.validate(getattr(value, name))
>  except ValidationError as error:
> @@ -510,6 +505,24 @@
>  return errors
>  
>  
> +class BindedSchema(object):

BoundSchema would be a more grammatically correct name.

> +"""This class proxies a schema to get its fields binded to a context

s/binded/bound/

> +"""
> +

[Zope-dev] Request for comment on zope.schema patch for bug 969350

2012-03-30 Thread Alexandre Garel

Hello,

I fill a bug report for zope.schema :

"zope.schema does not handle well nested object fields"
https://bugs.launchpad.net/zope.schema/+bug/969350

my patch change a particular behaviour (binding of Choice field nested 
in Object field)


I explain why on the ticket.

Any comment would be appreciated.

Alex

___
Zope-Dev maillist  -  Zope-Dev@zope.org
https://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists -
https://mail.zope.org/mailman/listinfo/zope-announce
https://mail.zope.org/mailman/listinfo/zope )