[dfulu]

Pull Request

Description

This PR cleans up our in-place operations

• We return None when an operation is done in-place to signify an in-place operation
• Updated docstrings and comments

Checklist:

• My code follows OCF's coding style guidelines
• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• I have checked my code and corrected any misspellings

[dfulu]

This function was a bit weird in that it replaced the data on the input dataarray and then did an isel(). When the input is an xarray.DataAray the isel returns a copy, so we were first changing the data in-place and then returning a copy of that changed data. That was kind of the worst of both worlds. This new version is explicitly out-of-place

[dfulu]

We do an in-place operation to load this so I don't think the name makes sense anymore

[felix-e-h-p]

Small suggestion. Could we have a test that pulls the same sample twice from PVNetDataset (with dropout and a fill value set) and checks that datasets_dict is unchanged? That would lock in the “without modifying the original NumPy arrays” from the new docstring.

[dfulu]

Small suggestion. Could we have a test that pulls the same sample twice from PVNetDataset (with dropout and a fill value set) and checks that datasets_dict is unchanged? That would lock in the “without modifying the original NumPy arrays” from the new docstring.

Sure, but I'd say that should be a unit test on preprocess_dataset_dict() rather than at the PVNetDataset level. The PVNetDataset is essentially only lazay arrays not NumPy arrays. So I think covering the lower level at preprocess_dataset_dict() is a better place for the test