VIP icon indicating copy to clipboard operation
VIP copied to clipboard

API: Handle HCIDataset modifying itself

Open r4lv opened this issue 6 years ago • 0 comments

This is related to a todo in HCIDataset.inject_companions: https://github.com/vortex-exoplanet/VIP/blob/acb112931b14f962f4eeede8774c0004b61c5d2c/vip_hci/hci_dataset.py#L778

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 = ...` internally

but in-place modification of attributes could cause problems:

d_copy.cube += 2  # ouch, modifies `d.cube` too

copy.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() and HCIDataset.deepcopy(), or refer to the copy module 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?

r4lv avatar Aug 01 '18 12:08 r4lv