Consistency in lazy setter methods

[javierggt]

The following tests would fail in master:

• self.q needs to be reset if self.transform or self.equatorial are set:

  q = np.array([[0.1227878,  0.69636424,  0.1227878, -0.69636424],
                [-0.23929834, -0.18930786,  0.03813458,  0.95154852]])
  q1 = Quat(q[0])
  q2 = Quat(q[1])
  q2.transform = q1.transform
  assert np.allclose(q2.transform, q1.transform)
  assert np.allclose(q2.q, q1.q)  # self.q must be reset, this fails

• self.q[-1] should always be greater or equal than zero:

  def test_lazy_set_flip():
  q = Quat([ 10,  90, -30])
  assert q.q[-1] >= 0 . # this fails

These failures come about as a consequence of inconsistencies in the lazy setters:

• _set_q resets self.transform and self.equatorial to None, but _set_equatorial and _settransform do not do the same. _set_q makes sure the scalar component of q is positive (by convention), but it is not called from _set_equatorial and _set_transform. self._q is changed directly instead.

Note: resetting to None is the right behavior when first setting the attribute, but not when lazy-setting it. A possible solution is to give the lazy-setters a 'reset' argument which defaults to true.

An illustration: Setting self._equatorial = None inside _set_q (current behavior) and replacing self._q by self.q inside _get_q (to fix the first issue above) leads to the following:

    q = Quat([ 10,  90, -30])
    assert np.allclose(q.q, np.array([-0.1227878 , -0.69636424, -0.1227878 ,  0.69636424]))
    assert np.allclose(q.equatorial, [ 10,  90, -30]) . # self.equatorial changed in the previous statement

This is not dramatic, since it happens in the singular point of equatorial coordinates.

[javierggt]

@taldcroft, the last "illustration" is the reason why we saw the weird behavior of creating a Quat with equatorial and right away checking it to see something different. You actually get this if you use tab-complete for q.equatorial:

In [11]: q = Quat([ 10,  90, -30])
In [12]: q.equatorial
Out[12]: array([ 161.56505118,   89.99999915,  180.        ])

[taldcroft]

@javierggt - interesting, good digging.

I think that (despite the plain fact that those setters are there), I never thought of something like q1.equatorial = q2.equatorial as being part of the public API. I wonder if there is any external code that does this. In any case, your idea of adding a reset=True argument seems fine. So the idea is that internal code would call e.g. self._set_equatorial(equatorial, reset=False) in places where it knows that it should not reset q or transform.

In some real sense the Quat instance should be immutable once created because you cannot guarantee consistency of the 3 representations otherwise. So we should probably make the internal attributes _q etc be read-only numpy arrays to prevent stuff like q.q[2] = 5.