[ase-users] VASP module in ASE

[Jens Jørgen Mortensen]

On Mon, 2008-09-01 at 14:07 +0200, Jakob Schiotz wrote:
> On Monday 01 September 2008, Jens Jørgen Mortensen wrote:
> 
> Just a few comments to Jens Jørgen's comments
> 
> >  class Vasp:
> > -    def __init__(self, restart=False, **kwargs):
> > +    def __init__(self, restart=None, **kwargs):
> >
> > ??? use None for default values.
> >
> 
> I think for default values one should either use None to mark it as a default, 
> or (if possible and not too messy) the actual value that will be used as 
> default.  Then it is more clear what the default is.  
> 
> >          if self.input_parameters['xc'] not in ['91','lda','pbe']:
> > -            raise Exception, "%s not supported for xc! use one of
> > ['91',lda','pbe']" % kwargs['xc']
> > -        self.positions=None
> > -        self.nbands=self.incar_parameters['nbands']
> > -        self.atoms=None
> > +            raise Exception(
> > +                "%s not supported for xc! use one of ['91',lda','pbe']"
> > %
> > +                kwargs['xc'])
> >
> > ??? use the syntax Exception('text')
> 
> Is there a reason for this?  I have always been using the other syntax :-)

You should start using the new syntax:

http://www.python.org/dev/peps/pep-3109/

> By the way, shouldn't we avoid exceptions of type Exception, this is the 
> baseclass for all exceptions and is probably best left alone and used only 
> for that purpose.  Instead one can use RuntimeError unless something else 
> like ValueError or TypeError seems more appropriate.

Yes, you are right.  So, in this case ValueError would be right.

> > @@ -117,9 +119,7 @@
> >              elif self.incar_parameters.has_key(key):
> >                  self.incar_parameters[key]=kwargs[key]
> >              else:
> > -                print 'Parameter '+key+' not defined'
> > -                return
> > -
> > +                raise TypeError('Parameter not defined: ' + key)
> >
> > ??? A wrong key should raise an exception.
> 
> But I think this should technically be a ValueError.  The variable key does 
> not have the wrong type, just the wrong value.

I agree!

> Just my 2 cents.
> 
> /Jakob
> 
>