amend sample to return copy and align weight axis
State: closed

Two bug fixes:
- Makes sure to return copies, not views. 
- makes sure if a separate series is passed as weights that the axis are aligned before arguments get converted into numpy arrays and handled positionally

closes #10736 


Labels:
name: API Design
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
Just wondering, is it needed to force a copy? Or could it also be an option that by default is True (eg `copy=True`), so that for cases where you never assign to a sample but only do some calculations on it you can indicate to not take a copy if possible for perf reasons?

That seems fine to me. I'll update shortly unless there are any objections.

On Mon, Aug 10, 2015 at 2:11 PM Joris Van den Bossche <
notifications@github.com> wrote:

> Just wondering, is it needed to force a copy? Or could it also be an
> option that by default is True (eg copy=True), so that for cases where
> you never assign to a sample but only do some calculations on it you can
> indicate to not take a copy if possible for perf reasons?
> 
> —
> Reply to this email directly or view it on GitHub
> https://github.com/pydata/pandas/pull/10738#issuecomment-129607081.

yep, I can buy @jorisvandenbossche suggestion.

@jreback  @shoyer 
Huh -- I'm a tad confused by the following: looks like I get a `SettingWithCopyWarning` when assigning values on a `df` returned from `sample()`, but the object being returned from `sample` isn't a view. 

```
In[21]:
df = pd.DataFrame({'col1':[5,6,7], 'col2':['a','b','c']})
a_sample = df.sample(1)
a_sample._is_view

Out[21]: False

In[22]:
a_sample['col1'] = 'z'
/Users/Nick/anaconda/envs/pandas_dev/lib/python3.4/site-packages/spyderlib/widgets/externalshell/start_ipython_kernel.py:1: SettingWithCopyWarning: 
A value is trying to be set on a copy of a slice from a DataFrame.
Try using .loc[row_indexer,col_indexer] = value instead

See the the caveats in the documentation: http://pandas.pydata.org/pandas-docs/stable/indexing.html#indexing-view-versus-copy
  # -*- coding: utf-8 -*-
```

And since I'm not passing any arguments, `sample` is basically just picking random values and passing them to `take` to pull rows out of `df`.

Given that, I think `take()` is just setting off the warning itself:

```
In[23]:
df = pd.DataFrame({'col1':[5,6,7], 'col2':['a','b','c']})
df.take([0,1])['col1'] = 2

/Users/Nick/anaconda/envs/pandas_dev/lib/python3.4/site-packages/spyderlib/widgets/externalshell/start_ipython_kernel.py:1: SettingWithCopyWarning: 
A value is trying to be set on a copy of a slice from a DataFrame.
Try using .loc[row_indexer,col_indexer] = value instead
```

It even happens for mono-typed `DataFrames`:

```
In[24]:
df = pd.DataFrame({'col1':[5,6,7], 'col2':[1,2,3]})
df.take([0,1])['col1'] = 2

/Users/Nick/anaconda/envs/pandas_dev/lib/python3.4/site-packages/spyderlib/widgets/externalshell/start_ipython_kernel.py:1: SettingWithCopyWarning: 
A value is trying to be set on a copy of a slice from a DataFrame.
Try using .loc[row_indexer,col_indexer] = value instead
```

Is that a bug with `take()`? Sorry, views / copies really throw me for a loop... 

@nickeubank 

Its a bit more complicated. `df.take` returns a copy (of the data underlying), and so I mark this `.is_copy`. So that if someone does a chained expression (which is what `df.take([0,1])['col1'] = 2` IS. We can figure this out.

```
In [23]: df = pd.DataFrame({'col1':[5,6,7], 'col2':[1,2,3]})

In [24]: foo = df.take([0,1])

In [25]: foo.is_copy 
Out[25]: <weakref at 0x1089dc050; to 'DataFrame' at 0x108974890>

In [28]: foo2 = df.take([0,1],is_copy=False)

In [29]: foo2.is_copy
```

so pass `is_copy=False` to avoid this problem.

Its not an issue of whether its a view or not (it never is), but you also have to turn off the 'marking' (which is a just a `weakref` back to the original data.

@jreback whoa. Ok. I think I get it. Kinda. :)

So if `.take()` always returns a copy, then I don't need to add a "copy or view" option to `sample`? Just write:

```
return self.take(locs, is_copy=False)
```

actually you might not need to do anything 
.sample() will always be correctly marked as a copy so nothing to do

what was the issue again?

I looked at he original issue
I think u could safe just set is_copy=False and this would work 

the confusion is if someone expects that the ORIGINAL frame would somehow be modified by setting a column in the sample - u see the difficulty here when people chain index

Haha -- things coming out of `sample()` were generating the `SettingWithCopyWarning` when modified. I thought that meant they were views, and so wanted to modify to return always copies. That's one of the two goals for this PR. 

(The other is index alignment). 

It is a bit about what people expect, but _I personally_ would not expect that I can change things in the result of `df.sample()` and have this modify `df` as well. So I think it is good in this case to not mark it as a copy (what you now have done).

But maybe you can add a note about this in the docstring that `sample` always returns a copy of the data.

@jorisvandenbossche  Docs amended. 

@jreback tweaked!

@nickeubank pls rebase and update for comments above. We always align things like this. So pls reindex. 

@jreback Ok! 

thanks!