On Thu, Jun 14, 2012 at 02:13:55PM -0700, Edward Pilatowicz wrote:
> On Mon, Jun 11, 2012 at 09:21:15AM -0700, Shawn Walker wrote:
> > On 06/10/12 22:57, Edward Pilatowicz wrote:
> > >On Tue, Jun 05, 2012 at 04:43:59PM -0700, Shawn Walker wrote:
> > >>On 05/22/12 01:45, Edward Pilatowicz wrote:
> > >>----------------------------------------
> > >>src/modules/misc2.py
> > >>----------------------------------------
> > >>   lines 250, 514: I don't see where 'type' gets assigned, and I
> > >>don't think we should have a variable named the same as a builtin
> > >>function either.
> > >>
> > >
> > >'type' doesn't get assigned anywhere.  this is a reference to the builtin.
> >
> > Then I'm super-confused :-/
> >
> > I think there are more bugs here then:
> >
> >  250         if type(desc) == type:
> >
> > For exampleL
> >
> > >>> print type("foo")
> > <type 'str'>
> > >>> print type("foo") == type
> > False
> > >>> print type("foo") is type
> > False
> >
> > In short, I'm struggling to figure out what this code is supposed to do.
> >
>
> the data descriptions format supported by json_{encode|decode} can
> contain objects and types.  this code is there to detect when the data
> description contains a type.  for example, in plandesc.py we have:
>
>       __state__desc = {
>               ...
>               "_rm_aliases": { str: set() },
>
> so when processing an _rm_aliases entry, the first thing we'll do is
> determine it's type by doing the following:
>
> ---8<---
> >>> type({ str: set() })
> <type 'dict'>
> ---8<---
>
> then we know we're dealing with a dictionary.  next, we'll try to
> determine the type of data used for keys in the dictionary, so we'll do:
>
> ---8<---
> >>> type(str)
> <type 'type'>
> ---8<---
>
> and that's where this check applies.  in this case the data description
> already contains a type, so we don't want to invoke type() on it.
>
> we could avoid this situation by not allowing types in the data
> descriptions, but i figured that allowing 'str' in the data description
> was more readable than requiring something like '""'.  (it also means
> that things like 'list' can be used in place of '[]', 'set' in place of
> 'set()', etc.)  it just makes things easier for consumers of the interface.

after talking about this in my office, we decided that the code would be
much easier to understand if written like this:

        # check if the description is a type object
        if isinstance(desc, type):
                desc_type = desc
        else:
                # get the expected data type from the description
                desc_type = type(desc)

ed
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to