sets data index if it's not None
State: closed

Fixed Bug #16342 read_stata ignores `index` parameter

Labels:
name: IO Stata
description: read_stata, to_stata

Comments:
Can you add a test for this? 
(that's always needed when fixing bus (or when adding new things as well), and typically you want to do first: write a test that currently fails, and then can check if with your changes the test passes)
There is already some functionality to do this in the `StataReader.read` method (but this does not honor the global passed keyword), so it would be nice to combine the fix with that.
Actually, have a look at this: https://github.com/pandas-dev/pandas/blob/master/pandas/io/stata.py#L1465-L1466
This is how it is done for `columns`, so you can do similar for `index`
Tests can go in `pandas/tests/io/test_stata.py`
I'm on it.

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16422?src=pr&el=h1) Report
> Merging [#16422](https://codecov.io/gh/pandas-dev/pandas/pull/16422?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 `50%`.

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

```diff
@@            Coverage Diff             @@
##           master   #16422      +/-   ##
==========================================
- Coverage   90.42%   90.42%   -0.01%     
==========================================
  Files         161      161              
  Lines       51023    51025       +2     
==========================================
  Hits        46138    46138              
- Misses       4885     4887       +2
```

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

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/16422?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/stata.py](https://codecov.io/gh/pandas-dev/pandas/pull/16422?src=pr&el=tree#diff-cGFuZGFzL2lvL3N0YXRhLnB5) | `93.4% <50%> (-0.08%)` | :arrow_down: |
| [pandas/core/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/16422?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY29tbW9uLnB5) | `91.05% <0%> (-0.34%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16422?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/16422?src=pr&el=footer). Last update [49ec31b...87a2de3](https://codecov.io/gh/pandas-dev/pandas/pull/16422?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

I am trying to create a test in test_strata before another pull request.  I will do that tomorrow morning.

Mo

Sent from my iPhone

> On May 22, 2017, at 5:34 PM, Jeff Reback <notifications@github.com> wrote:
> 
> @jreback requested changes on this pull request.
> 
> pls add the test from the issue & a whatsnew note.
> 
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 

I looked at https://docs.google.com/spreadsheets/d/13Xdux8j966QK6QKQtQIyyuS-dOriJvf6uH8LoZyvwYQ/edit#gid=1307138735
It's checking in StataParser.read
if columns is None:
            columns = self._columns

How does this help testing?    I'm new to this code; please give me some idea how to test it.  I was looking at test_stata.py and am confused about why "index" doesn't work anyway.  Here are my changes:  

```diff
def read_stata(filepath_or_buffer, convert_dates=True,
-               convert_categoricals=True, encoding=None, index=None,
+               convert_categoricals=True, encoding=None, index_col=None,
                convert_missing=False, preserve_dtypes=True, columns=None,
                order_categoricals=True, chunksize=None, iterator=False):
 
     reader = StataReader(filepath_or_buffer,
                          convert_dates=convert_dates,
                          convert_categoricals=convert_categoricals,
-                         index=index, convert_missing=convert_missing,
+                         index=index_col, convert_missing=convert_missing,
                          preserve_dtypes=preserve_dtypes,
                          columns=columns,
                          order_categoricals=order_categoricals,
@@ -178,8 +178,8 @@ def read_stata(filepath_or_buffer, convert_dates=True,
             data = reader.read()
         finally:
             reader.close()
-    if index is not None:
-        data.set_index(index)
+    if index_col is not None:
+        data.set_index(index_col)
     return data
```
@mogillies I'll push a test to this branch in a few minutes.
@mogillies ok just pushed a test. You can run it with `pytest pandas/tests/io/test_stata.py -k test_read_index` (it will fail right now)
I wrote this in test_stata.py and passed the test:
    def test_index_col(self):
        df = tm.makeDataFrame()
        df.index.name = 'index_col'
        reader = lambda x: read_stata(x).set_index('index_col')
        result = tm.round_trip_localpath(df.to_stata, reader)
        tm.assert_frame_equal(df, result)

in my branch.

That test doesn't quite do what we want. We want the user to be able to just do


```python
pd.read_stata(path, index_col="my_index"
```

In your test your `reader` first reads in the data with `read_stata` then calls `set_index('index_col')`.
So I am ready to commit and the test passed; but realize if someone call 
   result = pd.read_stata(path, iterator=True, index_col='my_index')
                                                ^^^^^^^^^^
it will fail the test with the following error:

E           AttributeError: 'StataReader' object has no attribute 'set_index'

Would you like me to commit and PR and is there an issue raised already?


@mogillies You will have to try to fix that error here as well in this PR, as this is related to your current fix.

Repeating what I linked to above: I think you should take a look at https://github.com/pandas-dev/pandas/blob/master/pandas/io/stata.py#L1465-L1466. There in the read function, the `columns` keyword is overridden with the one passed through `read_stata`, you can do the same for `index` 
can you rebase and update?
closing as stale