BUG: .iloc[:] and .loc[:] return a copy of the original object #13873
State: closed

Previously they returned the original. This PR should make it consistent with numpy and list.

 - [x] closes #13873
 - [x] tests added / passed
 - [x] passes ``git diff upstream/master --name-only -- '*.py' | flake8 --diff``
 - [x] whatsnew entry

Labels:
name: Bug
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves

Comments:
pls run the entire asv suite. I think you are copying unecessarily in a few places. This needs to be a *very* narrow change.

IOW, *only* when a copy is specify requested on the entire DataFrame, e.g. ``df.iloc[:]`` or ``df.loc[:]``. NOT anyother time as they return view by-definition.
@jreback Okay, I can revert the change for the tuple cases. I was going off of this comment on the original issue https://github.com/pandas-dev/pandas/issues/13873#issuecomment-236717538 
@margaret yes, this is *only* for all null slices passed. You have to be *very* careful with copying as that can really change performance. We always return views.
I think we should probably only be shallow copying in these cases (`.copy(deep=False)`)  - seems to be consistent with numpy semantics.

```
a = np.array([1, 2, 3])

a[:] is a
Out[44]: False

In [46]: a[:].base is a
Out[46]: True
```
Ran all the tests locally (but only on Python 2.7). Currently failing two tests: [`test_sorting.py`](https://github.com/pandas-dev/pandas/blob/master/pandas/tests/series/test_sorting.py#L84) for `Series` that was implemented for issue https://github.com/pandas-dev/pandas/issues/5856 which was fixed with PR https://github.com/pandas-dev/pandas/issues/5853

```python
        # GH 5856/5853
        # Series.sort_values operating on a view
        df = DataFrame(np.random.randn(10, 4))
        s = df.iloc[:, 0]

        def f():
            s.sort_values(inplace=True)

        pytest.raises(ValueError, f)
```
and something in `check_round_trip_frame` in [`test_clipboard.py`](https://github.com/pandas-dev/pandas/blob/master/pandas/tests/io/test_clipboard.py#L69)
```python
    def check_round_trip_frame(self, data_type, excel=None, sep=None,
                               encoding=None):
        data = self.data[data_type]
        data.to_clipboard(excel=excel, sep=sep, encoding=encoding)
        if sep is not None:
            result = read_clipboard(sep=sep, index_col=0, encoding=encoding)
        else:
            result = read_clipboard(encoding=encoding)
        tm.assert_frame_equal(data, result, check_dtype=False)
```
which raises
```
E       AssertionError: DataFrame are different
E       
E       DataFrame shape mismatch
E       [left]:  (2, 2)
E       [right]: (0, 1)

pandas/util/testing.py:1109: AssertionError
```

Looking into these.
The clipboard one is a flaky test, I'd ignore it - other failure probably is legitimate.

Can you also run the performance benchmarks to see if anything was impacted? Docs here:
http://pandas.pydata.org/pandas-docs/stable/contributing.html#running-the-performance-test-suite
Yep, looks like it is just the sorting a view test that's failing.
Calling `df.iloc[:,0]`, down in 
```
-> s = df.iloc[:, 0]
  /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(1325)__getitem__()
-> return self._getitem_tuple(key)
  /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(1664)_getitem_tuple()
-> return self._getitem_lowerdim(tup)
> /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(992)_getitem_lowerdim()
-> return getattr(section, self.name)[new_key]
```
when parsing the `0`, it ends up calling and returning `iloc[:]` https://github.com/margaret/pandas/blob/bb1b9c90e43a1ffcc61d4a0056e6439eb57502ea/pandas/core/indexing.py#L992
So that would return a copy of the `section` (the column) as a new object instead of the original column which here was actually a view into the ndarray.
Python 3.6 tests passed locally. Running benchmarks ⏳ 
Travis failure is on `ci/lint.sh`, but `git diff upstream/master --name-only -- '*.py' | flake8 --diff` passes, going to try running that locally.
Also the benchmarks are about 2/3 done now (have to go soon but will resume later), but there have been 3 failures (below). Going to look into that but if anyone has thoughts please share.
```
[  0.00%] · For pandas commit hash b904308f:
...
[ 33.22%] ··· Running panel_methods.PanelMethods.time_pct_change_items 
[ 33.29%] ··· Running panel_methods.PanelMethods.time_pct_change_major
[ 33.36%] ··· Running panel_methods.PanelMethods.time_pct_change_minor
```
asv benchmark results in this [gist](https://gist.github.com/margaret/489f8b5a8e6e8eb754a01df0c425dbb7). 🤔 
@margaret a couple (very tiny) linting errors :) https://travis-ci.org/pandas-dev/pandas/jobs/235411163

```
pandas/tests/indexing/test_iloc.py:599:16: E714 test for object identity should be 'is not'
pandas/tests/indexing/test_loc.py:638:16: E714 test for object identity should be 'is not'
```
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=h1) Report
> Merging [#16443](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/49ec31bbaeca81a6f58fc1be26fe80f3ac188cdd?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `80%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/16443/graphs/tree.svg?token=eZ4WkYLtcO&src=pr&width=650&height=150)](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #16443      +/-   ##
==========================================
- Coverage   90.42%   90.42%   -0.01%     
==========================================
  Files         161      161              
  Lines       51023    51029       +6     
==========================================
+ Hits        46138    46143       +5     
- Misses       4885     4886       +1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.26% <80%> (-0.01%)` | :arrow_down: |
| #single | `40.17% <40%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/indexing.py](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhpbmcucHk=) | `93.92% <80%> (-0.09%)` | :arrow_down: |
| [pandas/core/reshape/merge.py](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZXJnZS5weQ==) | `94.18% <0%> (-0.01%)` | :arrow_down: |
| [pandas/errors/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree#diff-cGFuZGFzL2Vycm9ycy9fX2luaXRfXy5weQ==) | `100% <0%> (ø)` | :arrow_up: |
| [pandas/core/missing.py](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvbWlzc2luZy5weQ==) | `84.27% <0%> (ø)` | :arrow_up: |
| [pandas/core/reshape/reshape.py](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9yZXNoYXBlLnB5) | `99.28% <0%> (ø)` | :arrow_up: |
| [pandas/io/formats/style.py](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvc3R5bGUucHk=) | `95.92% <0%> (+0.01%)` | :arrow_up: |
| [pandas/io/excel.py](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree#diff-cGFuZGFzL2lvL2V4Y2VsLnB5) | `62.31% <0%> (+0.05%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=footer). Last update [49ec31b...d80bab4](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=h1) Report
> Merging [#16443](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/2e24a8f8886a1481af8324d44224cddb2ee1b53c?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `80%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/16443/graphs/tree.svg?height=150&width=650&token=eZ4WkYLtcO&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #16443      +/-   ##
==========================================
+ Coverage   90.91%   90.94%   +0.02%     
==========================================
  Files         161      161              
  Lines       49284    49286       +2     
==========================================
+ Hits        44809    44822      +13     
+ Misses       4475     4464      -11
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.71% <80%> (+0.02%)` | :arrow_up: |
| #single | `40.17% <40%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/indexing.py](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhpbmcucHk=) | `93.92% <80%> (-0.09%)` | :arrow_down: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `65.05% <0%> (+1.81%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=footer). Last update [2e24a8f...d130824](https://codecov.io/gh/pandas-dev/pandas/pull/16443?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

both loc and iloc should return shallow copies for [:] 
Indexing with a single loc or iloc call otherwise should always return a view, right? In v0.21.0 `df.iloc[:,0]` on a df created from a 2d ndarray returns a different object than `df[0]`. So is that actually incorrect behavior?
> Indexing with a single loc or iloc call otherwise should always return a view, right? In v0.21.0 df.iloc[:,0] on a df created from a 2d ndarray returns a different object than df[0]. So is that actually incorrect behavior?

can you show a specific example.
```
>>> pd.__version__
'0.20.1'
>>> df = pd.DataFrame(np.random.randn(10, 4))
>>> df.loc[:,0] is df[0]
True
>>> df.iloc[:,0] is df[0]
False
```
@margaret hmm, those should both be views. was this changed from 0.19.2?
Nope, looks like it's the same in 0.19.2. I think it's [this line where Series.from_array is called](https://github.com/pandas-dev/pandas/blob/master/pandas/core/frame.py#L2095) in `DataFrame._ixs` that's creating a new object, since `section` [here](https://github.com/pandas-dev/pandas/blob/master/pandas/core/indexing.py#L967) is what's being returned. Weird though since default for `from_array` is `copy=False`

```
-> df.iloc[:,0]
  /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(1330)__getitem__()
-> return self._getitem_tuple(key)
  /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(1669)_getitem_tuple()
-> return self._getitem_lowerdim(tup)
  /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(968)_getitem_lowerdim()
-> section = self._getitem_axis(key, axis=i)
  /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(1756)_getitem_axis()
-> return self._get_loc(key, axis=axis)
  /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(139)_get_loc()
-> return self.obj._ixs(key, axis=axis)
> /Users/margaret/Documents/Projects/pandas/pandas/core/frame.py(2096)_ixs()
-> result = self._constructor_sliced.from_array(values,
```

edit: Hm, looks like `loc` here ultimately just calls `df[0]`
```
-> df.loc[:,0]
  /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(1329)__getitem__()
-> return self._getitem_tuple(key)
  /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(836)_getitem_tuple()
-> return self._getitem_lowerdim(tup)
  /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(967)_getitem_lowerdim()
-> section = self._getitem_axis(key, axis=i)
  /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(1556)_getitem_axis()
-> return self._get_label(key, axis=axis)
  /Users/margaret/Documents/Projects/pandas/pandas/core/indexing.py(136)_get_label()
-> return self.obj._xs(label, axis=axis)
> /Users/margaret/Documents/Projects/pandas/pandas/core/generic.py(1932)xs()
-> return self[key]
```

yeah we prob don't systematically check this
welcome for u to open a new issue / PR for that
@margaret lgtm. can you add the same tests for ``df[:]`` and ``s[:]`` they should do the same.
CircleCI failed with
```
Suppressing install of ssh keys for fork PR builds
fatal: No such section!
fatal: No such section!
```
on 'start container' and then when it tried ssh-ing into the VM.  Could someone try re-running it? Or maybe there's a setting that needs to be updated?
@jreback
looks reasonable. pls rebase and push again to have the CI run fully.

ping on green.
@jreback ✅ 
thanks @margaret nice fix. sometimes it takes a while to get these in. keep em coming!