Basically, do we prefer an 'out' kwarg or an 'inplace' version?

On Sun, May 4, 2014 at 5:22 PM, James Booth <notifications@github.com>
wrote:

>> @@ -66,7 +66,7 @@ def principal_component_decomposition(X, whiten=False, center=True,
>>      else:
>>          mean_vector = np.zeros(n_features)
>>  
>> -    X -= mean_vector
>> +    X = X - mean_vector
> so there are two separate issues:
> 1. How is `data` constructed in PCAModel? does it involve copying? Will it change with #301?
> 2. How is PCA taken on data with `principal_component_decomposition`? Is there any way #301 could mean the vectorized objects used to build `data` are modified in place? 
> Taking the first one...
> For our cases, a **numpy array is always one chunk of memory**. So when we do
> ```python
> data = np.zeros((n_samples, n_features))
> ```
> We make a new chunk of memory. If I slice into this array and assign:
> ```python
> data[:, 0] = y
> ```
> it doesn't matter if `y` is a view onto another array (as it will be after #301) or is it's own array - **assignment into a slice will always copy the array into the specified slice**. This has to be the case, otherwise it would invalidate the above rule.
> So what difference will #301 make to building `data`? 
> In that for loop you pointed out:
> - Pre #301 we made a copy of each image's vector and then copied this vector into a slice in `data`. 
> - After #301 a view will be taken and then copied into a slice in `data`. 
> This difference is minor - the memory overhead pre-301 was just a single copy of an image throughout that loop. After data has been constructed **there is no difference**. `data` owns it's own memory in both cases.
> Answering 2 is now obvious - there will be no change back to vectorized objects after #301 because data is always a copy. So what was going on before and after Patrick's patch?
> Before, the call to `principal_component_decomposition()` destructively changed `data` after the call. This wasn't an issue, because `data` was it's own array. After Patrick's patch, whilst in the `principal_component_decomposition()` method memory usage will double as we have to make a copy of `X` to avoid changing the input outside of the function (which generally is a good thing to avoid). However, this may be unacceptable due to memory size, hence my suggestion above that we add a kwarg to allow an inplace version of the function.
> Hope that clears it up! 
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/menpo/menpo/pull/305/files#r12262769