On Tue, May 20, 2014 at 07:35:25PM -0700, Richard West wrote:
> > @@ -618,13 +618,13 @@ def loadRecipe(self, actions):
> >              assert action[0] in ['CHANGE_BOND','FORM_BOND','BREAK_BOND','GAIN_RADICAL','LOSE_RADICAL','GAIN_PAIR','LOSE_PAIR']
> >              self.forwardRecipe.addAction(action)
> >  
> > -    def loadForbidden(self, label, group, shortDesc='', longDesc=''):
> > +    def loadForbidden(self, label, group, multiplicity = [1,2,3,4,5], shortDesc='', longDesc=''):
> 
> We'll *probably* get away with it here (we're *unlikely* to add or
> remove from the list), but we should avoid mutable default
> arguments…

I couldn't trace this down to where the multiplicity list is actually
used, but it's probably easier to just use a tuple.  Also, PEP 8
suggests [1]:

  Don't use spaces around the = sign when used to indicate a keyword
  argument or a default parameter value

so I'm for:

  def loadForbidden(self, label, group, multiplicity=(1,2,3,4,5), shortDesc='', longDesc=''):

The following note from the PR message would probably also be useful
in the loadForbidden doc string (and for other functions and methods
that take an iterable multiplicity):

  For groups the multiplicity label is always a separate argument
  (thermo and kinetics) and is defined as a list containing all
  accepted multiplicities where that group will be applicable.

[1]: http://legacy.python.org/dev/peps/pep-0008/#whitespace-in-expressions-and-statements