[spambayes-dev] RE: [Spambayes-checkins] spambayes/spambayes OptionsClass.py, 1.19, 1.20

[Kenny Pitt]

I was looking at this same code a little bit yesterday, and one thing
struck me as odd.  The get(), get_option(), and set() functions use
self.conversion_table to translate the requested option, but many of the
other functions such as display_name() don't.  Is there a reason for
that?  If not, I was wondering if it would be easier to just fix
get_option() for case-insensitivity and then have all the other getter
functions call it instead of accessing self._options directly.

Also note that the self.conversion_table translation in get() is already
redundant, as it then calls get_option() which will do the exact same
translation.

Tony Meyer wrote:
> Update of /cvsroot/spambayes/spambayes/spambayes
> In directory sc8-pr-cvs1:/tmp/cvs-serv9453/spambayes
> 
> Modified Files:
> 	OptionsClass.py
> Log Message:
> Option names are always case insensitive, no matter what.
> 
> Index: OptionsClass.py
> ===================================================================
> RCS file: /cvsroot/spambayes/spambayes/spambayes/OptionsClass.py,v
> retrieving revision 1.19
> retrieving revision 1.20
> diff -C2 -d -r1.19 -r1.20
> *** OptionsClass.py	15 Dec 2003 09:20:33 -0000	1.19
> --- OptionsClass.py	16 Dec 2003 04:48:28 -0000	1.20
> ***************
> *** 552,586 ****
>       def display_name(self, sect, opt):
>           '''A name for the option suitable for display to a user.'''
> !         return self._options[sect, opt].display_name()
>[snip]
>       def get_option(self, sect, opt):
> --- 552,586 ----
>       def display_name(self, sect, opt):
>           '''A name for the option suitable for display to a user.'''
> !         return self._options[sect, opt.lower()].display_name()
>[snip]
>       def get_option(self, sect, opt):
> ***************
> *** 588,598 ****
>           if self.conversion_table.has_key((sect, opt)):
>               sect, opt = self.conversion_table[sect, opt]
> !         return self._options[sect, opt]
> 
>       def get(self, sect, opt):
>           '''Get an option value.'''
> !         if self.conversion_table.has_key((sect, opt)):
> !             sect, opt = self.conversion_table[sect, opt]
> !         return self.get_option(sect, opt).get()
> 
>       def __getitem__(self, key):
> --- 588,598 ----
>           if self.conversion_table.has_key((sect, opt)):
>               sect, opt = self.conversion_table[sect, opt]
> !         return self._options[sect, opt.lower()]
> 
>       def get(self, sect, opt):
>           '''Get an option value.'''
> !         if self.conversion_table.has_key((sect, opt.lower())):
> !             sect, opt = self.conversion_table[sect, opt.lower()]
> !         return self.get_option(sect, opt.lower()).get()
> 
>       def __getitem__(self, key):
> ***************
> *** 601,612 ****
>       def set(self, sect, opt, val=None):
>           '''Set an option.'''
> !         if self.conversion_table.has_key((sect, opt)):
> !             sect, opt = self.conversion_table[sect, opt]
>           if self.is_valid(sect, opt, val):
> !             self._options[sect, opt].set(val)
>           else:
>               print >> sys.stderr, ("Attempted to set [%s] %s with
>                                     invalid" " value %s (%s)" %
> !                                   (sect, opt, val, type(val)))
> 
>       def set_from_cmdline(self, arg, stream=None):
> --- 601,612 ----
>       def set(self, sect, opt, val=None):
>           '''Set an option.'''
> !         if self.conversion_table.has_key((sect, opt.lower())):
> !             sect, opt = self.conversion_table[sect, opt.lower()]
>           if self.is_valid(sect, opt, val):
> !             self._options[sect, opt.lower()].set(val)
>           else:
>               print >> sys.stderr, ("Attempted to set [%s] %s with
>                                     invalid" " value %s (%s)" %
> !                                   (sect, opt.lower(), val,
> type(val))) 
> 
>       def set_from_cmdline(self, arg, stream=None):

-- 
Kenny Pitt