Re: [Freeipa-devel] [PATCH 0020] Add IntEnum parameter to ipalib

2013-10-09 Thread Petr Viktorin

On 10/08/2013 08:37 PM, Nathaniel McCallum wrote:

On Tue, 2013-10-08 at 18:29 +0200, Petr Viktorin wrote:

On 10/07/2013 11:28 PM, Nathaniel McCallum wrote:

On Mon, 2013-10-07 at 13:22 +0200, Petr Viktorin wrote:

On 10/04/2013 07:33 PM, Nathaniel McCallum wrote:

This patch is preparatory for the OTP CLI patch.



+def _convert_scalar(self, value, index=None):
+return Int._convert_scalar(self, value, index=index)

That won't work. In Python 2 unbound methods (such as
Int._validate_scalar) must be passed the correct type as self; passing
an IntEnum instance like this will raise a TypeError.

You'll need to either use multiple inheritance (if you feel the
framework isn't complex enough), or make a convert_int function, and
then in both Int and IntEnum just call it and handle ValueError.

For validate_scalar it would probably be best to extend
Param._validate_scalar to allow the class to define extra allowed types,
and get rid of the reimplementation in Int.


Fixed.


This works, but I do have some comments.

I'd prefer if the framework changes and IntEnum addition were in
separate patches.
I find the usage of _get_types() a bit convoluted, especially when you
need to get the canonical type. I've taken the liberty to do this with
an `allowed_types` attribute, which I think is easier to use, and also
doesn't break the API.
Would you agree with these changes?

I've added tests for IntEnum, as a combination of StrEnum and Int tests.
Do they look OK?


Everything looks great except I suspect the reworking of convert_int()
belongs in the second patch.


Makes sense, fixed.


--
Petr³
From 70f7a88774e9b2f1f148bfaeee2635d966c38342 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Mon, 30 Sep 2013 12:45:37 -0400
Subject: [PATCH] Allow multiple types in Param type validation

Int already needed to take both int and long. This makes the functionality
available for all Param classes.
---
 ipalib/parameters.py| 53 +++--
 ipatests/test_ipalib/test_parameters.py |  3 +-
 2 files changed, 20 insertions(+), 36 deletions(-)

diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 79b9062bb52c639ce72bc7d7fae19e7fba100e51..97e449c29902eb3ee1c2213a16a281fdc6ec62fc 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -363,7 +363,9 @@ class Param(ReadOnly):
 
 # This is a dummy type so that most of the functionality of Param can be
 # unit tested directly without always creating a subclass; however, a real
-# (direct) subclass must *always* override this class attribute:
+# (direct) subclass must *always* override this class attribute.
+# If multiple types are permitted, set `type` to the canonical type and
+# `allowed_types` to a tuple of all allowed types.
 type = NoneType  # Ouch, this wont be very useful in the real world!
 
 # Subclasses should override this with something more specific:
@@ -400,6 +402,11 @@ class Param(ReadOnly):
 # ('default', self.type, None),
 )
 
+@property
+def allowed_types(self):
+The allowed datatypes for this Param
+return (self.type,)
+
 def __init__(self, name, *rules, **kw):
 # We keep these values to use in __repr__():
 self.param_spec = name
@@ -415,7 +422,7 @@ def __init__(self, name, *rules, **kw):
 self.nice = '%s(%r)' % (self.__class__.__name__, self.param_spec)
 
 # Add 'default' to self.kwargs and makes sure no unknown kw were given:
-assert type(self.type) is type
+assert all(type(t) is type for t in self.allowed_types)
 if kw.get('multivalue', True):
 self.kwargs += (('default', tuple, None),)
 else:
@@ -782,7 +789,7 @@ def _convert_scalar(self, value, index=None):
 
 Convert a single scalar value.
 
-if type(value) is self.type:
+if type(value) in self.allowed_types:
 return value
 raise ConversionError(name=self.name, index=index,
 error=ugettext(self.type_error),
@@ -816,7 +823,7 @@ def validate(self, value, context=None, supplied=None):
 self._validate_scalar(value)
 
 def _validate_scalar(self, value, index=None):
-if type(value) is not self.type:
+if type(value) not in self.allowed_types:
 raise TypeError(
 TYPE_ERROR % (self.name, self.type, value, type(value))
 )
@@ -942,7 +949,7 @@ def _convert_scalar(self, value, index=None):
 
 Convert a single scalar value.
 
-if type(value) is self.type:
+if type(value) in self.allowed_types:
 return value
 if isinstance(value, basestring):
 value = value.lower()
@@ -1009,7 +1016,7 @@ def _convert_scalar(self, value, index=None):
 
 Convert a single scalar value.
 
-if type(value) is self.type:
+if type(value) in 

Re: [Freeipa-devel] [PATCH 0020] Add IntEnum parameter to ipalib

2013-10-09 Thread Nathaniel McCallum
On Wed, 2013-10-09 at 10:23 +0200, Petr Viktorin wrote:
 On 10/08/2013 08:37 PM, Nathaniel McCallum wrote:
  On Tue, 2013-10-08 at 18:29 +0200, Petr Viktorin wrote:
  On 10/07/2013 11:28 PM, Nathaniel McCallum wrote:
  On Mon, 2013-10-07 at 13:22 +0200, Petr Viktorin wrote:
  On 10/04/2013 07:33 PM, Nathaniel McCallum wrote:
  This patch is preparatory for the OTP CLI patch.
 
 
  +def _convert_scalar(self, value, index=None):
  +return Int._convert_scalar(self, value, index=index)
 
  That won't work. In Python 2 unbound methods (such as
  Int._validate_scalar) must be passed the correct type as self; passing
  an IntEnum instance like this will raise a TypeError.
 
  You'll need to either use multiple inheritance (if you feel the
  framework isn't complex enough), or make a convert_int function, and
  then in both Int and IntEnum just call it and handle ValueError.
 
  For validate_scalar it would probably be best to extend
  Param._validate_scalar to allow the class to define extra allowed types,
  and get rid of the reimplementation in Int.
 
  Fixed.
 
  This works, but I do have some comments.
 
  I'd prefer if the framework changes and IntEnum addition were in
  separate patches.
  I find the usage of _get_types() a bit convoluted, especially when you
  need to get the canonical type. I've taken the liberty to do this with
  an `allowed_types` attribute, which I think is easier to use, and also
  doesn't break the API.
  Would you agree with these changes?
 
  I've added tests for IntEnum, as a combination of StrEnum and Int tests.
  Do they look OK?
 
  Everything looks great except I suspect the reworking of convert_int()
  belongs in the second patch.
 
 Makes sense, fixed.

Two smaller issues.

You define allowed_types as a @property in Param and then as a tuple in
Int. This seems stylistically inconsistent, though I understand why
you've done this, it breaks a certain understanding about the behavior
of subclasses of Int.

Also, in IntEnum, you've set IntEnum.types rather than
IntEnum.allowed_types.

Nathaniel




___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0020] Add IntEnum parameter to ipalib

2013-10-09 Thread Petr Viktorin

On 10/09/2013 04:17 PM, Nathaniel McCallum wrote:

On Wed, 2013-10-09 at 10:23 +0200, Petr Viktorin wrote:

On 10/08/2013 08:37 PM, Nathaniel McCallum wrote:

On Tue, 2013-10-08 at 18:29 +0200, Petr Viktorin wrote:

On 10/07/2013 11:28 PM, Nathaniel McCallum wrote:

On Mon, 2013-10-07 at 13:22 +0200, Petr Viktorin wrote:

On 10/04/2013 07:33 PM, Nathaniel McCallum wrote:

This patch is preparatory for the OTP CLI patch.



 +def _convert_scalar(self, value, index=None):
 +return Int._convert_scalar(self, value, index=index)

That won't work. In Python 2 unbound methods (such as
Int._validate_scalar) must be passed the correct type as self; passing
an IntEnum instance like this will raise a TypeError.

You'll need to either use multiple inheritance (if you feel the
framework isn't complex enough), or make a convert_int function, and
then in both Int and IntEnum just call it and handle ValueError.

For validate_scalar it would probably be best to extend
Param._validate_scalar to allow the class to define extra allowed types,
and get rid of the reimplementation in Int.


Fixed.


This works, but I do have some comments.

I'd prefer if the framework changes and IntEnum addition were in
separate patches.
I find the usage of _get_types() a bit convoluted, especially when you
need to get the canonical type. I've taken the liberty to do this with
an `allowed_types` attribute, which I think is easier to use, and also
doesn't break the API.
Would you agree with these changes?

I've added tests for IntEnum, as a combination of StrEnum and Int tests.
Do they look OK?


Everything looks great except I suspect the reworking of convert_int()
belongs in the second patch.


Makes sense, fixed.


Two smaller issues.

You define allowed_types as a @property in Param and then as a tuple in
Int. This seems stylistically inconsistent, though I understand why
you've done this, it breaks a certain understanding about the behavior
of subclasses of Int.


I don't think that's much of an issue. Using a @property that would 
always return the same value seems redundant.

What understanding did you have in mind?


Also, in IntEnum, you've set IntEnum.types rather than
IntEnum.allowed_types.


Fixed, thanks.

--
Petr³

From 70f7a88774e9b2f1f148bfaeee2635d966c38342 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Mon, 30 Sep 2013 12:45:37 -0400
Subject: [PATCH] Allow multiple types in Param type validation

Int already needed to take both int and long. This makes the functionality
available for all Param classes.
---
 ipalib/parameters.py| 53 +++--
 ipatests/test_ipalib/test_parameters.py |  3 +-
 2 files changed, 20 insertions(+), 36 deletions(-)

diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 79b9062bb52c639ce72bc7d7fae19e7fba100e51..97e449c29902eb3ee1c2213a16a281fdc6ec62fc 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -363,7 +363,9 @@ class Param(ReadOnly):
 
 # This is a dummy type so that most of the functionality of Param can be
 # unit tested directly without always creating a subclass; however, a real
-# (direct) subclass must *always* override this class attribute:
+# (direct) subclass must *always* override this class attribute.
+# If multiple types are permitted, set `type` to the canonical type and
+# `allowed_types` to a tuple of all allowed types.
 type = NoneType  # Ouch, this wont be very useful in the real world!
 
 # Subclasses should override this with something more specific:
@@ -400,6 +402,11 @@ class Param(ReadOnly):
 # ('default', self.type, None),
 )
 
+@property
+def allowed_types(self):
+The allowed datatypes for this Param
+return (self.type,)
+
 def __init__(self, name, *rules, **kw):
 # We keep these values to use in __repr__():
 self.param_spec = name
@@ -415,7 +422,7 @@ def __init__(self, name, *rules, **kw):
 self.nice = '%s(%r)' % (self.__class__.__name__, self.param_spec)
 
 # Add 'default' to self.kwargs and makes sure no unknown kw were given:
-assert type(self.type) is type
+assert all(type(t) is type for t in self.allowed_types)
 if kw.get('multivalue', True):
 self.kwargs += (('default', tuple, None),)
 else:
@@ -782,7 +789,7 @@ def _convert_scalar(self, value, index=None):
 
 Convert a single scalar value.
 
-if type(value) is self.type:
+if type(value) in self.allowed_types:
 return value
 raise ConversionError(name=self.name, index=index,
 error=ugettext(self.type_error),
@@ -816,7 +823,7 @@ def validate(self, value, context=None, supplied=None):
 self._validate_scalar(value)
 
 def _validate_scalar(self, value, index=None):
-if type(value) is not self.type:
+if type(value) not in self.allowed_types:
 

Re: [Freeipa-devel] [PATCH 0020] Add IntEnum parameter to ipalib

2013-10-09 Thread Nathaniel McCallum
On Wed, 2013-10-09 at 16:36 +0200, Petr Viktorin wrote:
 On 10/09/2013 04:17 PM, Nathaniel McCallum wrote:
  On Wed, 2013-10-09 at 10:23 +0200, Petr Viktorin wrote:
  On 10/08/2013 08:37 PM, Nathaniel McCallum wrote:
  On Tue, 2013-10-08 at 18:29 +0200, Petr Viktorin wrote:
  On 10/07/2013 11:28 PM, Nathaniel McCallum wrote:
  On Mon, 2013-10-07 at 13:22 +0200, Petr Viktorin wrote:
  On 10/04/2013 07:33 PM, Nathaniel McCallum wrote:
  This patch is preparatory for the OTP CLI patch.
 
 
   +def _convert_scalar(self, value, index=None):
   +return Int._convert_scalar(self, value, index=index)
 
  That won't work. In Python 2 unbound methods (such as
  Int._validate_scalar) must be passed the correct type as self; passing
  an IntEnum instance like this will raise a TypeError.
 
  You'll need to either use multiple inheritance (if you feel the
  framework isn't complex enough), or make a convert_int function, and
  then in both Int and IntEnum just call it and handle ValueError.
 
  For validate_scalar it would probably be best to extend
  Param._validate_scalar to allow the class to define extra allowed 
  types,
  and get rid of the reimplementation in Int.
 
  Fixed.
 
  This works, but I do have some comments.
 
  I'd prefer if the framework changes and IntEnum addition were in
  separate patches.
  I find the usage of _get_types() a bit convoluted, especially when you
  need to get the canonical type. I've taken the liberty to do this with
  an `allowed_types` attribute, which I think is easier to use, and also
  doesn't break the API.
  Would you agree with these changes?
 
  I've added tests for IntEnum, as a combination of StrEnum and Int tests.
  Do they look OK?
 
  Everything looks great except I suspect the reworking of convert_int()
  belongs in the second patch.
 
  Makes sense, fixed.
 
  Two smaller issues.
 
  You define allowed_types as a @property in Param and then as a tuple in
  Int. This seems stylistically inconsistent, though I understand why
  you've done this, it breaks a certain understanding about the behavior
  of subclasses of Int.
 
 I don't think that's much of an issue. Using a @property that would 
 always return the same value seems redundant.
 What understanding did you have in mind?

I don't have a better option. It just stood out to me.

  Also, in IntEnum, you've set IntEnum.types rather than
  IntEnum.allowed_types.
 
 Fixed, thanks.

I think this is ready to merge. Do you?

Nathaniel


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0020] Add IntEnum parameter to ipalib

2013-10-09 Thread Petr Viktorin

On 10/09/2013 05:44 PM, Nathaniel McCallum wrote:

On Wed, 2013-10-09 at 16:36 +0200, Petr Viktorin wrote:

On 10/09/2013 04:17 PM, Nathaniel McCallum wrote:

On Wed, 2013-10-09 at 10:23 +0200, Petr Viktorin wrote:

On 10/08/2013 08:37 PM, Nathaniel McCallum wrote:

On Tue, 2013-10-08 at 18:29 +0200, Petr Viktorin wrote:

On 10/07/2013 11:28 PM, Nathaniel McCallum wrote:

On Mon, 2013-10-07 at 13:22 +0200, Petr Viktorin wrote:

On 10/04/2013 07:33 PM, Nathaniel McCallum wrote:

This patch is preparatory for the OTP CLI patch.



  +def _convert_scalar(self, value, index=None):
  +return Int._convert_scalar(self, value, index=index)

That won't work. In Python 2 unbound methods (such as
Int._validate_scalar) must be passed the correct type as self; passing
an IntEnum instance like this will raise a TypeError.

You'll need to either use multiple inheritance (if you feel the
framework isn't complex enough), or make a convert_int function, and
then in both Int and IntEnum just call it and handle ValueError.

For validate_scalar it would probably be best to extend
Param._validate_scalar to allow the class to define extra allowed types,
and get rid of the reimplementation in Int.


Fixed.


This works, but I do have some comments.

I'd prefer if the framework changes and IntEnum addition were in
separate patches.
I find the usage of _get_types() a bit convoluted, especially when you
need to get the canonical type. I've taken the liberty to do this with
an `allowed_types` attribute, which I think is easier to use, and also
doesn't break the API.
Would you agree with these changes?

I've added tests for IntEnum, as a combination of StrEnum and Int tests.
Do they look OK?


Everything looks great except I suspect the reworking of convert_int()
belongs in the second patch.


Makes sense, fixed.


Two smaller issues.

You define allowed_types as a @property in Param and then as a tuple in
Int. This seems stylistically inconsistent, though I understand why
you've done this, it breaks a certain understanding about the behavior
of subclasses of Int.


I don't think that's much of an issue. Using a @property that would
always return the same value seems redundant.
What understanding did you have in mind?


I don't have a better option. It just stood out to me.


Also, in IntEnum, you've set IntEnum.types rather than
IntEnum.allowed_types.


Fixed, thanks.


I think this is ready to merge. Do you?


Thanks! Let's get it in.
Pushed to master: 5e8aab8558874a9a826a1c470e806c75fb84eef2

--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0020] Add IntEnum parameter to ipalib

2013-10-08 Thread Petr Viktorin

On 10/07/2013 11:28 PM, Nathaniel McCallum wrote:

On Mon, 2013-10-07 at 13:22 +0200, Petr Viktorin wrote:

On 10/04/2013 07:33 PM, Nathaniel McCallum wrote:

This patch is preparatory for the OTP CLI patch.



   +def _convert_scalar(self, value, index=None):
   +return Int._convert_scalar(self, value, index=index)

That won't work. In Python 2 unbound methods (such as
Int._validate_scalar) must be passed the correct type as self; passing
an IntEnum instance like this will raise a TypeError.

You'll need to either use multiple inheritance (if you feel the
framework isn't complex enough), or make a convert_int function, and
then in both Int and IntEnum just call it and handle ValueError.

For validate_scalar it would probably be best to extend
Param._validate_scalar to allow the class to define extra allowed types,
and get rid of the reimplementation in Int.


Fixed.


This works, but I do have some comments.

I'd prefer if the framework changes and IntEnum addition were in 
separate patches.
I find the usage of _get_types() a bit convoluted, especially when you 
need to get the canonical type. I've taken the liberty to do this with 
an `allowed_types` attribute, which I think is easier to use, and also 
doesn't break the API.

Would you agree with these changes?

I've added tests for IntEnum, as a combination of StrEnum and Int tests. 
Do they look OK?


--
Petr³

From b475201139705f6f166aa6354e0685b0a272b00f Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Mon, 30 Sep 2013 12:45:37 -0400
Subject: [PATCH] Allow multiple types in Param type validation

Int already needed to take both int and long. This makes the functionality
available for all Param classes.
---
 ipalib/parameters.py| 98 +
 ipatests/test_ipalib/test_parameters.py |  3 +-
 2 files changed, 41 insertions(+), 60 deletions(-)

diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 79b9062bb52c639ce72bc7d7fae19e7fba100e51..6ac6cb278b61adb0b039e0293a2696846debc24c 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -363,7 +363,9 @@ class Param(ReadOnly):
 
 # This is a dummy type so that most of the functionality of Param can be
 # unit tested directly without always creating a subclass; however, a real
-# (direct) subclass must *always* override this class attribute:
+# (direct) subclass must *always* override this class attribute.
+# If multiple types are permitted, set `type` to the canonical type and
+# `allowed_types` to a tuple of all allowed types.
 type = NoneType  # Ouch, this wont be very useful in the real world!
 
 # Subclasses should override this with something more specific:
@@ -400,6 +402,11 @@ class Param(ReadOnly):
 # ('default', self.type, None),
 )
 
+@property
+def allowed_types(self):
+The allowed datatypes for this Param
+return (self.type,)
+
 def __init__(self, name, *rules, **kw):
 # We keep these values to use in __repr__():
 self.param_spec = name
@@ -415,7 +422,7 @@ def __init__(self, name, *rules, **kw):
 self.nice = '%s(%r)' % (self.__class__.__name__, self.param_spec)
 
 # Add 'default' to self.kwargs and makes sure no unknown kw were given:
-assert type(self.type) is type
+assert all(type(t) is type for t in self.allowed_types)
 if kw.get('multivalue', True):
 self.kwargs += (('default', tuple, None),)
 else:
@@ -782,7 +789,7 @@ def _convert_scalar(self, value, index=None):
 
 Convert a single scalar value.
 
-if type(value) is self.type:
+if type(value) in self.allowed_types:
 return value
 raise ConversionError(name=self.name, index=index,
 error=ugettext(self.type_error),
@@ -816,7 +823,7 @@ def validate(self, value, context=None, supplied=None):
 self._validate_scalar(value)
 
 def _validate_scalar(self, value, index=None):
-if type(value) is not self.type:
+if type(value) not in self.allowed_types:
 raise TypeError(
 TYPE_ERROR % (self.name, self.type, value, type(value))
 )
@@ -942,7 +949,7 @@ def _convert_scalar(self, value, index=None):
 
 Convert a single scalar value.
 
-if type(value) is self.type:
+if type(value) in self.allowed_types:
 return value
 if isinstance(value, basestring):
 value = value.lower()
@@ -1009,7 +1016,7 @@ def _convert_scalar(self, value, index=None):
 
 Convert a single scalar value.
 
-if type(value) is self.type:
+if type(value) in self.allowed_types:
 return value
 if type(value) in (unicode, int, long, float):
 try:
@@ -1030,13 +1037,29 @@ class Int(Number):
 
 
 type = int
+allowed_types = int, long
 type_error = 

Re: [Freeipa-devel] [PATCH 0020] Add IntEnum parameter to ipalib

2013-10-08 Thread Nathaniel McCallum
On Tue, 2013-10-08 at 18:29 +0200, Petr Viktorin wrote:
 On 10/07/2013 11:28 PM, Nathaniel McCallum wrote:
  On Mon, 2013-10-07 at 13:22 +0200, Petr Viktorin wrote:
  On 10/04/2013 07:33 PM, Nathaniel McCallum wrote:
  This patch is preparatory for the OTP CLI patch.
 
 
 +def _convert_scalar(self, value, index=None):
 +return Int._convert_scalar(self, value, index=index)
 
  That won't work. In Python 2 unbound methods (such as
  Int._validate_scalar) must be passed the correct type as self; passing
  an IntEnum instance like this will raise a TypeError.
 
  You'll need to either use multiple inheritance (if you feel the
  framework isn't complex enough), or make a convert_int function, and
  then in both Int and IntEnum just call it and handle ValueError.
 
  For validate_scalar it would probably be best to extend
  Param._validate_scalar to allow the class to define extra allowed types,
  and get rid of the reimplementation in Int.
 
  Fixed.
 
 This works, but I do have some comments.
 
 I'd prefer if the framework changes and IntEnum addition were in 
 separate patches.
 I find the usage of _get_types() a bit convoluted, especially when you 
 need to get the canonical type. I've taken the liberty to do this with 
 an `allowed_types` attribute, which I think is easier to use, and also 
 doesn't break the API.
 Would you agree with these changes?
 
 I've added tests for IntEnum, as a combination of StrEnum and Int tests. 
 Do they look OK?

Everything looks great except I suspect the reworking of convert_int()
belongs in the second patch.

Nathaniel


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0020] Add IntEnum parameter to ipalib

2013-10-07 Thread Petr Viktorin

On 10/04/2013 07:33 PM, Nathaniel McCallum wrote:

This patch is preparatory for the OTP CLI patch.



 +def _convert_scalar(self, value, index=None):
 +return Int._convert_scalar(self, value, index=index)

That won't work. In Python 2 unbound methods (such as 
Int._validate_scalar) must be passed the correct type as self; passing 
an IntEnum instance like this will raise a TypeError.


You'll need to either use multiple inheritance (if you feel the 
framework isn't complex enough), or make a convert_int function, and 
then in both Int and IntEnum just call it and handle ValueError.


For validate_scalar it would probably be best to extend 
Param._validate_scalar to allow the class to define extra allowed types, 
and get rid of the reimplementation in Int.


--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0020] Add IntEnum parameter to ipalib

2013-10-07 Thread Nathaniel McCallum
On Mon, 2013-10-07 at 13:22 +0200, Petr Viktorin wrote:
 On 10/04/2013 07:33 PM, Nathaniel McCallum wrote:
  This patch is preparatory for the OTP CLI patch.
 
 
   +def _convert_scalar(self, value, index=None):
   +return Int._convert_scalar(self, value, index=index)
 
 That won't work. In Python 2 unbound methods (such as 
 Int._validate_scalar) must be passed the correct type as self; passing 
 an IntEnum instance like this will raise a TypeError.
 
 You'll need to either use multiple inheritance (if you feel the 
 framework isn't complex enough), or make a convert_int function, and 
 then in both Int and IntEnum just call it and handle ValueError.
 
 For validate_scalar it would probably be best to extend 
 Param._validate_scalar to allow the class to define extra allowed types, 
 and get rid of the reimplementation in Int.

Fixed.


From 9820b610bc9877e06d5e6cfb40138f37fcc8ab3b Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Mon, 30 Sep 2013 12:45:37 -0400
Subject: [PATCH] Add IntEnum parameter to ipalib

---
 ipalib/__init__.py  |   2 +-
 ipalib/parameters.py| 130 
 ipatests/test_ipalib/test_parameters.py |   2 +-
 3 files changed, 69 insertions(+), 65 deletions(-)

diff --git a/ipalib/__init__.py b/ipalib/__init__.py
index d822ba5956d6afb6ef6d88063f8359926e47016b..ab89ab77ec94603d242e56436021c9b6ed8663cb 100644
--- a/ipalib/__init__.py
+++ b/ipalib/__init__.py
@@ -886,7 +886,7 @@ from frontend import Command, LocalOrRemote, Updater, Advice
 from frontend import Object, Method, Property
 from crud import Create, Retrieve, Update, Delete, Search
 from parameters import DefaultFrom, Bool, Flag, Int, Decimal, Bytes, Str, IA5Str, Password, DNParam, DeprecatedParam
-from parameters import BytesEnum, StrEnum, AccessTime, File
+from parameters import BytesEnum, StrEnum, IntEnum, AccessTime, File
 from errors import SkipPluginModule
 from text import _, ngettext, GettextFactory, NGettextFactory
 
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index f75057f1e0f87c5bd12a617def69c5c9cd267bd7..945cdedaae21a5c4e95bb7ffd1e96c60c6f08cf5 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -361,7 +361,9 @@ class Param(ReadOnly):
 
 # This is a dummy type so that most of the functionality of Param can be
 # unit tested directly without always creating a subclass; however, a real
-# (direct) subclass must *always* override this class attribute:
+# (direct) subclass must *always* override this class attribute. If multiple
+# types are permitted, they may be specified in a tuple. In this case, the
+# canonical type is the first type listed.
 type = NoneType  # Ouch, this wont be very useful in the real world!
 
 # Subclasses should override this with something more specific:
@@ -396,6 +398,16 @@ class Param(ReadOnly):
 # ('default', self.type, None),
 )
 
+def _get_types(self):
+if type(self.type) is tuple:
+return self.type
+
+if type(self.type) is type:
+return (self.type,)
+
+assert False
+
+
 def __init__(self, name, *rules, **kw):
 # We keep these values to use in __repr__():
 self.param_spec = name
@@ -416,7 +428,7 @@ class Param(ReadOnly):
 self.nice = '%s(%r)' % (self.__class__.__name__, self.param_spec)
 
 # Add 'default' to self.kwargs and makes sure no unknown kw were given:
-assert type(self.type) is type
+assert not filter(lambda t: not type(t) is type, self._get_types())
 if kw.get('multivalue', True):
 self.kwargs += (('default', tuple, None),)
 else:
@@ -787,7 +799,7 @@ class Param(ReadOnly):
 
 Convert a single scalar value.
 
-if type(value) is self.type:
+if type(value) in self._get_types():
 return value
 raise ConversionError(name=self.name, index=index,
 error=ugettext(self.type_error),
@@ -821,7 +833,7 @@ class Param(ReadOnly):
 self._validate_scalar(value)
 
 def _validate_scalar(self, value, index=None):
-if type(value) is not self.type:
+if type(value) not in self._get_types():
 raise TypeError(
 TYPE_ERROR % (self.name, self.type, value, type(value))
 )
@@ -924,7 +936,7 @@ class Param(ReadOnly):
 json_dict[a] = json_serialize(val)
 json_dict['class'] = self.__class__.__name__
 json_dict['name'] = self.name
-json_dict['type'] = self.type.__name__
+json_dict['type'] = self._get_types()[0].__name__
 return json_dict
 
 
@@ -947,7 +959,7 @@ class Bool(Param):
 
 Convert a single scalar value.
 
-if type(value) is self.type:
+if type(value) in self._get_types():
 return value
 if isinstance(value, basestring):
 value =