-
Notifications
You must be signed in to change notification settings - Fork 63
Description
This is related to a todo in HCIDataset.inject_companions:
Line 778 in acb1129
| # TODO: return array/HCIDataset object instead? |
current situation
For now, HCIDataset.inject_companions(...) modifies the self.cube.
proposition
Most of the methods of HCIDataset actually modify its state, e.g. .filter(), .drop_frames(), .derotate(), so I think we should not return a new HCIDataset object or array.
However, we could introduce a new .copy() method, which would return a copy of the HCIDataset. That copy could then be used for injection etc., and the original HCIDataset could still be kept.
thoughts on HCIDataset.copy()
Copying a HCIDataset can be done with the standard library's copy.copy() or copy.deepcopy(). copy.copy() uses the same references for the attributes, so no additional memory is used. That works well when "replacing" the attributes (= the most cases):
d = HCIDataset()
d_copy = copy.copy(d) # could be implemented as d.copy()
d_copy.cube = d_copy.cube + 2 # okay
d_copy.inject_companions() # okay, as calls `self.cube = ...` internallybut in-place modification of attributes could cause problems:
d_copy.cube += 2 # ouch, modifies `d.cube` toocopy.deepcopy() really copies every attribute in memory, so the original and copied object are completely separated. The downside is the increased memory usage.
copy.deepcopy() also lets "user-defined classes override the copying operation or the set of components copied".
Whe should think of
- will the user actually use in-place operations? Or is it safe to
copy.copy()? - should we create two methods,
HCIDataset.copy()andHCIDataset.deepcopy(), or refer to thecopymodule in the docs, so the user has the choice (and needs to know the difference)? - should we be on the safe side and use
copy.deepcopy()internally, and sacrifice memory? - shoudl we deepcopy the small attributes, like angles and PSF, but copy the cube to save memory?