Safer BaseCoordinateFrame?
- Dominant language
- Python
- Stars
- 5.3k
- Forks
- 2.2k
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 74
Description
Dear `astropy` developers,
first let me thank you for the great job you have done here :) I have been using the `astropy.coordinates` package for some time now and find it very convenient. Yet I have a few concerns which I discuss below.
The intent of `BaseCoordinateFrame` w.r.t. mutability is not clear to me. On one side the class restricts changing `self.data` (by not implementing the corresponding property setter) but on the other side the low level data are `numpy.ndarray` which are actually mutable. Furthermore, the class uses as cache which can get invalid without any notice. This can lead to strange bugs, for example as following:
```python
>>> from numpy import zeros
>>> from astropy.coordinates import BaseCoordinateFrame, CartesianRepresentation
>>> z2 = zeros(2)
>>> frame = BaseCoordinateFrame(CartesianRepresentation(z2, z2, z2))
>>> frame.spherical.distance # This gets cached
>>> frame.cartesian.x[:] += 1 # Someone modifies the coordinates in-place
>>> frame.spherical.distance # The cached value is used
```
Explicitly clearing the cache (e.g. with `self.cache.clear()`) before the 2nd call to `frame.spherical` would solve the problem in this specific case. However, in a less obvious situation where the two calls are largely separated (e.g. in different functions implemented by different users) this can get really messy and error prone.
In order to prevent these types of errors a simple solution to me would be an option to disable the cache. This would allow for a more robust, yet less efficient, frame version.
Another confusing point to me is that in the previous example `frame.cartesian` while differing from the `frame.data` representation actually gets references to the data instead of copies. For example:
```python
>>> frame.cartesian is frame.data
False
>>> frame.cartesian.x is frame.data.x
True
```
To me it would be safer if the representation like attributes of a frame (`frame.cartesian`, `frame.spherical`, ...) would always return a copy of the data, instead of references when matching the data representation. If one explicitly wants a reference to the coordinates data then there is the `frame.data` property whose name clearly indicates that one is manipulating the initial data. Though, such a change could have a significant impact on the performances of existing codes :(
Contributor guide
Research direction
Start with astropy.coordinates.BaseCoordinateFrame and the cache, data, cartesian, and spherical entry points shown in the examples; no file or test is named in the issue. A complete change would need an agreed mutability and cache design, with tests showing that in-place edits cannot leave returned values inconsistent.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- numpy, python
- Domain
- backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100