search

lkilcher / dolfyn

A library for oceanographic doppler instruments such as Acoustic Doppler Profilers (ADPs, ADCPs) and Acoustic Doppler Velocimeters (ADVs).
BSD 3-Clause "New" or "Revised" License
42 stars 25 forks source link

Add 'inplace' ability to rotate.api functions #78

Closed jmcvey3 closed 2 years ago

jmcvey3 commented 2 years ago

Namely rotate2, set_declination, and set_inst2head_rotmat. I set the default value as True, and added these functions to the velds attribute. Also made sure that particular functions that return a dataset are creating a new dataset to keep the original input intact.

lkilcher commented 2 years ago

Do we need a note in the functions that have the inplace option along the lines of:

When making these kinds of inplace changes to the data, this only changes the data in memory not the data on disk. You will need to use a .save method in order to write these changes to disk.

Or is this obvious/unnecessary? ... I just don't know how much xarray users expect the data in memory to be identifcal to what is on disk.

jmcvey3 commented 2 years ago

Do we need a note in the functions that have the inplace option along the lines of:

When making these kinds of inplace changes to the data, this only changes the data in memory not the data on disk. You will need to use a .save method in order to write these changes to disk.

Or is this obvious/unnecessary? ... I just don't know how much xarray users expect the data in memory to be identifcal to what is on disk.

If you open up a netcdf file through xarray, anything you do to it must be manually saved; it's no different from a word doc: if you don't save your progress and power goes out you'll lose it. The "inplace" is more for convenience and speed than anything, though I suppose it saves memory as well by not needing to create a new dataset.

lkilcher commented 2 years ago

Great! In that case I think this PR is almost ready to merge. The only Q I have is whether there is a way to 'run the checks' here before we do so?