ENH: Add support for excluding the index from Parquet files (GH20768)
State: closed

- [x] closes #20768
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Labels:
name: Enhancement
name: IO Parquet
description: parquet, feather

Comments:
Hello @dargueta! Thanks for updating the PR.







Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on August 21, 2018 at 18:00 Hours UTC
Good start!  Going to need tests as well as a `whatsnew` entry (potentially a mini-section).

cc @jreback 
> Going to need tests as well as a whatsnew entry (potentially a mini-section).

I won't have much time until later today but yeah, I'll finish that off!
@gfyoung I'm not sure I completely understand what's going on in the unit tests. I _think_ I've written a test that handles the expected cases for both engines but I'm not sure. I've never contributed to Pandas before. 😆 
Do you have any idea what's causing these seemingly unrelated tests to fail on Travis? Like, my code should not be failing because "snappy compression is unavailable."

The failing appveyor test seems to be a bit more relevant but I don't fully understand it. It seems to not like the multi-index when read back even though I'm explicitly excluding it from consideration:

```
E       DataFrame.index classes are not equivalent
E       [left]:  MultiIndex(levels=[['bar', 'baz', 'foo', 'qux'], ['one', 'two']],
E                  labels=[[0, 0, 1, 1, 2, 2, 3, 3], [0, 1, 0, 1, 0, 1, 0, 1]])
E       [right]: RangeIndex(start=0, stop=8, step=1)
pandas\util\testing.py:1076: AssertionError
```

These tests pass just fine on CircleCI, which is weird.
Just glancing at the Travis failures on 3.6 they seem related - looks like there is a difference in the name of the index before / after (`None` vs `index`)
So why isn't it failing on CircleCI? Isn't it the same code?

Also, my changes should _not_ be causing `TestPythonParser.test_no_header_prefix` to suddenly start failing, right? That's another one failing on Travis.
@dargueta : Good question!  Not quite.  The coverage for each build / platform is slightly different.  That's why we use multiple platforms, for better or worse. 🙂 
Okay, good to know. Is there a straightforward way to ignore the index when loading it back, considering we're deliberately not writing it?
Remind me: why can't you pass in an argument for `read_kwargs` ?
~I meant for testing, not actual production.~

~This feature was intended to be only for writing, but I suppose I could add it for reading? Not sure how well that'd be supported by the Parquet libraries but I can check. Unless I'm misunderstanding your question?~

Ignore that, I misunderstood the question. I think I fixed the issue, but [other tests](https://travis-ci.org/pandas-dev/pandas/jobs/415746873#L3124) are still failing.
> Only not fully sure what the best solution is here. We could restore the current behaviour (not write index by default for a default index, i.e. write_index=None)

The problem is, `pyarrow` _always_ writes the index. Different engines already have different behavior. So technically this change alters the existing `fastparquet` behavior.

I suppose we could have _three_ values, `None` to use the default behavior for the engine, and then `True` or `False` for explicit control.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22266?src=pr&el=h1) Report
> Merging [#22266](https://codecov.io/gh/pandas-dev/pandas/pull/22266?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/1c113db60b68c5a262d64e92dc9de72bfe59aed5?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `90%`.

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

```diff
@@            Coverage Diff             @@
##           master   #22266      +/-   ##
==========================================
+ Coverage   92.17%   92.18%   +<.01%     
==========================================
  Files         169      169              
  Lines       50778    50781       +3     
==========================================
+ Hits        46807    46810       +3     
  Misses       3971     3971
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.59% <90%> (ø)` | :arrow_up: |
| #single | `42.32% <20%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22266?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/22266/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.19% <ø> (ø)` | :arrow_up: |
| [pandas/io/parquet.py](https://codecov.io/gh/pandas-dev/pandas/pull/22266/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhcnF1ZXQucHk=) | `73.72% <90%> (+0.68%)` | :arrow_up: |

------

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

Last of the (so far) requested fixes are in!
> why is there a `None` option here?

`fastparquet` doesn't write the index if it's an integer sequence 0-n, but `pyarrow` always writes the index by default. To preserve backwards compatibility, there's a `None` option that maintains the original engine-dependent behavior.
> fastparquet doesn't write the index if it's an integer sequence 0-n, but pyarrow always writes the index by default. To preserve backwards compatibility, there's a None option that maintains the original engine-dependent behavior.

I c.

This makes fp not idempotent though, which is not a great situation here. I wonder if should make ``index=True/False`` the only option, deprecating ``None`` to preserve back compat with fp. Of course this would make it show on practically every use. 
> This makes fp not idempotent though, which is not a great situation here

Do you mean that it would not round-trip faithfully? 
Because that is still the case, as it only does not write the index for a default, non-named index. So when reading such parquet file without an explicit index, the created index will be no different.
> Do you mean that it would not round-trip faithfully?

is there some magic metadata that it does? The problem is that this round-trip is *only* good for fp-fp?

I think we should standardize on the pandas side with ``index=True``

> I think we should standardize on the pandas side with `index=True`

That's technically an abrupt backwards-incompatible change for anyone using `fastparquet` to export dataframes for use by another consumer such as a database.

Personally, for minimal disruption I believe we should release this feature with `None` as the default, with an explicit note that `True` will be the default in the next backwards-incompatible release.
@jreback and @jorisvandenbossche is there anything else you think needs to be addressed?
can you rebase
@jreback rebase is done
@dargueta Thanks a lot!