[ase-users] Atoms.set_calculator() deprecated?

[Jakob Schiøtz]

Dear Ask,

Thank you very much for your thorough answer!


> On 20 May 2020, at 19.15, Ask Hjorth Larsen <asklarsen at gmail.com> wrote:
> 
> Dear Jakob,
> 
> Am Mi., 20. Mai 2020 um 18:03 Uhr schrieb Jakob Schiøtz via ase-users
> <ase-users at listserv.fysik.dtu.dk>:
>> 
>> Hi all,
>> 
>> What was the reason for deprecating Atoms.set_calculator() in favor of just setting the .calc attribute?  It is still a function that is being called.
> 
> Having only one is better than having both.  We regularly take some
> flak for having multiple ways of doing the same thing.  So I suggested
> it in an issue which got 5 thumbs up, which is the current high score.

Yes, I agree with the principles of only having one way to do stuff.  And the attributes are in some way more natural.  But they don't play nice with inheritance.  And they break the Zen of Python, "explicit is better than implicit" by implicitly calling a function when you think you set a property.


>> And more importantly: How the !€#% do I call that "function" (which is not a function, but a property) of the Atoms class, when I have a subclass that now has to overload the .calc property but needs to call the original not-quite-a-function.  With functions that is easy, but my Python-fu is insufficient to do this with properties.  I end up with infinite recursions no matter what I try.
> 
> Huh, that's kind of strange.  It isn't exactly user-friendly.  I
> rummaged around and got to this:
> 
> from ase import Atoms
> 
> class MyAtoms(Atoms):
>    @property
>    def calc(self):
>        print('fget')
>        return Atoms.calc.fget(self)
> 
>    @calc.setter
>    def calc(self, calc):
>        print('fset')
>        Atoms.calc.fset(self, calc)
> 
> atoms = MyAtoms()
> atoms.calc = 'hello'
> print(atoms.calc)
> 
> The ugly part being that you'd expect properties to be kind of
> transparent, rather than requiring subclasses to *know* that they're
> properties.

The properties live on the instances, not on the classes, in some way that I do not fully understand.  But they are not really compatible with inheritance (and thus object oriented programming).

Originally they were added as a hack, so you could add functionality when setting something that should have been set by a setter, but was instead just an attribute.  Along the way we have been using it, or in my opinion abusing it, to make an interface that looks simpler but really just hides that it is complex.

Or maybe it shifts the burden of complexity from the users to the developers, which I guess is fine...

>> Or would anybody object to partly reverting this change, un-deprecate Atoms.set_calculator() and letting Atoms.calc be a property calling the get/set functions which actually do the work?
> 
> I think it might be okay to have getters/setters to help subclasses,
> but I prefer to still deprecate the old API.  As long as remains clear
> to users what the purpose of each thing is.

I'll try the ugly inheritance you suggested first.

> A real issue is that it's not clear what the subclassing rules are for
> Atoms.  Subclassing complex objects is always finicky, having a
> tendency to break abstractions that otherwise work.  So I always
> prefer delegation.

You mean that the ParallelAtoms object in Asap should behave like an atom by delegating every single method and attribute to the original atoms class.  Shudder!  That sounds like a true nightmare for a complex class like Atoms.

Or maybe you mean something altogether different, but I misunderstand you.  That would not be a first.  :-)


Best regards

Jakob