Preventing assignment isn't necessary for the internal workings of
SymPy. The only advantage it has is potentially avoiding confusion for
users. I don't particularly see users getting confused about this
though (although maybe that's because of the widespread use of
properties).
What I think is important is that any properties/attributes are
derivable from `.args` and not stored separately. Also I think it's
good to avoid using `.args` directly where possible so something like:
class MyThing(Basic):
def __new__(cls, one, another):
obj = Basic.__new__(cls, one, another)
@property
def one(self):
return self.args[0]
@property
def another(self):
return self.args[1]
def get_something(self):
return f(self.one) # Using one rather than .args[0]
Then apart from the properties none of the other methods should
directly access args. This makes it possible to easily subclass
MyThing and have a different args layout in the subclass. This is
common when creating a singleton subclass:
class MyThingZero(MyThin, Singleton):
def __new__(cls):
return Basic.__new__(cls)
@property
def one(self):
return 0
@property
def another(self):
return oo
Here MyThingZero doesn't need to have .args because its a singleton so
they couldn't mean anything. MyThingZero can inherit all of the
methods of MyThing and only needs to redefine the accessor properties
because nothing else touches .args directly.
Sometimes it is desirable to cache some property of the object as an
attribute in __new__ but this should only done if it is something
derivable from .args and actually I think it's better to do the
caching in a property.
class Bad(Basic):
def __new__(self, one, another):
obj = Basic.__new__(cls, another)
obj._one = one # <--------------- don't do this
This is bad because the `_one` attribute is stored outside of .args.
Once you've done this you've interrupted the machinery of Basic and
you'll end up being tempted to override __eq__, __hash__, .args,
.func, etc and there's no way you'll get all the details right so
there will be subtle bugs and things you haven't thought of like
pickling will break. You can try to fix these things one at a time but
by the time you're finished you will have reimplemented a good chunk
of Basic.
It might be desirable to cache an object as an attribute to avoid
recomputing it. As long as it is derived from .args that is okay:
class Okay(Basic)
def __new__(self, one, another):
obj = Basic.__new__(cls, one, another)
# This is derivable from args but expensive so we cache it
obj._attribute = expensive_function(one)
@property
def attribute(self):
return self._attribute
Better though is to put that caching inside the property so if we
don't need it then expensive_function is never called:
class Better(Basic):
def __new__(self, one, another):
obj = Basic.__new__(cls, one, another)
@property
def attribute(self):
val = getattr(self, '_attribute', None)
if val is not None:
return val
else:
val = self._attribute = expensive_function(self.one)
return val
I'm not sure if there is already a decorator that does this kind of
caching (it should be added if there isn't).
The examples of bad things I show above are all ones that I've seen in
SymPy's own codebase so please judge for yourself what is reasonable
rather than looking for precedents in the existing code. Currently the
codebase is inconsistent and there are a lot of Basic subclasses doing
dodgy things with attributes and args. I would like to clean that up
though.
Oscar
On Sun, 8 Sep 2019 at 11:49, Rybalka Denys <rybalka.de...@gmail.com> wrote:
>
> There are several ways to implement properties of sympy objects. The simplest
> one is:
> ```
> def __new__(*args):
> obj = Basic(*args)
> obj.prop = 'some_property'
> ```
> A slightly more complicated one is:
> ```
> def __new__(*args):
> obj = Basic(*args)
> obj._prop = 'some_property'
>
> @property
> def prop(self):
> return self._prop
> ```
>
> The first method is simpler and shorter, but the second one explicitly
> forbids assignments `obj.prop = ...`. I have tried looking into various sympy
> modules and the practices are different. Which implementation is preferable?
> And on a more general topic, is there a "coding guidelines" webpage, where
> issues like this should be resolved once and for all? Maybe it makes sense to
> create one? I believe it will improve both the code and user experience.
>
> Best Regards,
> Denys Rybalka
>
> --
> You received this message because you are subscribed to the Google Groups
> "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to sympy+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/sympy/CAOsx_LqFFGo9wifF30QC8sT%3DYutaEcj6aYPNU7Tt154ZbPnD2w%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups
"sympy" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to sympy+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/sympy/CAHVvXxQzUPZTEai%3DCh0Y%2BKN%2BRRRQRu8P0pwpk9JLxFJL64DEJw%40mail.gmail.com.
