Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API: Handle HCIDataset modifying itself #266

Open
r4lv opened this issue Aug 1, 2018 · 0 comments
Open

API: Handle HCIDataset modifying itself #266

r4lv opened this issue Aug 1, 2018 · 0 comments

Comments

@r4lv
Copy link
Contributor

r4lv commented Aug 1, 2018

This is related to a todo in HCIDataset.inject_companions:

# 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 = ...` 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?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant