suggest making Angle immutable in Python

[TallJimbo]

Most numerical types in Python are immutable; this works well with the assignment behavior that just replaces a variable with another. You'll note that things that appear to work in-place still do (such as += operators), but they don't actually operate in-place at the C pointer level.

With that in mind, I think there will be advantages down the road to making our Angle class immutable in Python as well. This would just involve removing the in-place arithmetic operators from the wrappers. Python would then invoke the non-in-place versions of those operators for us, making them behave the same way they do for Python floats.

One example (though I think this is a good idea just in principle): I think this will help with using properties and other descriptors: I'm not sure if a true in-place operator invokes a setter function (which often has useful side effects), but the fall-back behavior Python uses when an in-place operator isn't present does invoke a setter function. For example, in shear, the "beta" property is supposed to be read-only, because we only define a getter function

class Shear(object):
    ...
    beta = property(getBeta)
    ...

But we can still do this:

s = Shear()
s.beta += 1.2

We may want to be able to do that, actually, but it should be explicit, by providing a setBeta to the property.

[rmjarvis]

I don't understand what the advantages would be to using the python version of += rather than the c++ version. Can you provide a concrete example of what doesn't work now (or works poorly in some sense) that would work better if we didn't wrap +=?

To be clear, I'm not averse to the change. I just don't understand the point of it.

[TallJimbo]

In the above example with the Shear object, the "property" line says essentially, "make this a read-only attribute":

class Shear(object):
    ...
    beta = property(getBeta)    # we've only provided a getter, not a setter
    ...

Because Angle defines a += operator, however, we're allowed to do this:

s = Shear()
s.beta += 1.2 * radians   # this evalutes to "s.getBeta() += 1.2 * radians" in C++, which modifies a temporary

... even though we're correctly not allowed to do this:

s.beta = 1.2 * radians

So it's a matter of raising an exception instead of allowing the user to think they've modified something when they have not.

I think it's fair to consider this a problem with how Python handles in-place arithmetic operators. In short, they work better if you don't define them. If we don't define an in-place operator for Angle, we get an exception above where we'd expect. But if we had defined the Shear property like this:

class Shear(object):
    ...
    beta = property(getBeta, setBeta)
    ...

...and not defined an in-place operator, then in-place assignment would still have worked, and done the right thing:

s = Shear()
s.beta += 1.2 * radians     # now evaluates to "s.setBeta(s.getBeta() + 1.2 * radians)"

In C++, that replacement would make in-place assignment less efficient, but it's totally trivial compared to the overhead in Python.

[rmjarvis]

I see. It's really because python doesn't have a const attribute. So you can't tell it that s is const, so s.beta's op+= shouldn't be accessible.

[rmandelb]

... do we have a conclusion on this and on the related issue for Shear? Or are we still waiting to see how the config stuff plays out? @barnabytprowe ?

[barnabytprowe]

Thanks for the prod Rachel :) It seems like there's a good argument for it, from what I've just read. And if it amounts solely to removing the inplace operators, that's a pretty easy change to put back if we find that this modification has unforseen / unwanted consequences...! So, I'm pro.

[rmandelb]

If there is agreement on this and the Shear issue (#220) then perhaps they could be done together on a branch. Neither of them is a huge change.

On Jul 26, 2012, at 5:24 PM, Barnaby Rowe wrote:

Thanks for the prod Rachel :) It seems like there's a good argument for it, from what I've just read. And if it amounts solely to removing the inplace operators, that's a pretty easy change to put back if we find that this modification has unforseen / unwanted consequences...! So, I'm pro.

---

Reply to this email directly or view it on GitHub:
#219 (comment)

---

Rachel Mandelbaum
rmandelb@andrew.cmu.edu

[barnabytprowe]

This issue is being closed due to work done on #148 and #241