DOC: Update parquet metadata format description around index levels
State: closed

cc @wesm @jcrist

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

Comments:
lgtm. merge when ready @cpcloud 
@cpcloud ?
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18201?src=pr&el=h1) Report
> Merging [#18201](https://codecov.io/gh/pandas-dev/pandas/pull/18201?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/9629fef912dbeca5154e9cf58970bad3e02657f4?src=pr&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `n/a`.

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

```diff
@@            Coverage Diff             @@
##           master   #18201      +/-   ##
==========================================
- Coverage   91.59%   91.55%   -0.04%     
==========================================
  Files         153      153              
  Lines       51257    51257              
==========================================
- Hits        46949    46929      -20     
- Misses       4308     4328      +20
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.41% <ø> (-0.03%)` | :arrow_down: |
| #single | `40.68% <ø> (-0.11%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/18201?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/18201/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/18201/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `64.78% <0%> (-1.74%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/18201/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.81% <0%> (-0.1%)` | :arrow_down: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/18201/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.59% <0%> (-0.1%)` | :arrow_down: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/18201/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `82.01% <0%> (+0.19%)` | :arrow_up: |

------

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

@cpcloud good to go?
I think the example at then end also should be updated: https://github.com/cpcloud/pandas/blame/6183078033adc62c1ccf043dac15d5b4f121fdac/doc/source/developer.rst#L157 (it still uses `'__index_level_0__'` in the `columns` part of the metadata, where the actual name (`None` in this case) should be included.
Having dived a little bit in the pyarrow side of the code in https://github.com/apache/arrow/pull/1386, I have two design-level comments or questions related to the index names:

- Currently there is no strict mapping possible (or at least harder than it should be) between the pandas metadata and the actual field names in the schema: because index column names in the `pandas_metadata['columns']` entry don't match the field name, and because the order / number of entries in the `pandas_metadata` is not always the same as the fields when subsetting data (so you can't just enumerate both together). 

  Therefore I was wondering whether it would be worth adding a `'field_name'` entry to a `pandas_metadata['columns']` entry (next to the `'name'` entry). `'field_name'` would then strictly match the names of the schema, making lookups much easier.

- The choice was made to always encode index columns in the schema with `__index_level_n__`-like names, even if the index (or index level) of the pandas DataFrame has a name that is not None. The actual name is still preserved in `pandas_metadata['columns'][..]['name']`, so it is not a problem for roundtripping. 
  But for usability of pyarrow itself, I would much prefer having a field with the actual name and not `__index_level_n__`. Also in pandas we are trying in some places to reduce the difference between index levels and columns (seeing the index as a special column), eg in groupby, merging, sorting you can now mix references of columns and index levels by name. 
  This change was done IIRC to accomodate index level names that were the same as column names, but I would personally just regard that the same as duplicate column names (thus: not supported).

I *think* those changes are not yet in a released version of pyarrow? (and the last one is also not yet implemented in fastparquet I think to align with pyarrow)
Well, we are running out of time to make any more such changes for Arrow 0.8.0. What about using the disambiguated names only when there is a naming conflict?
cc @cpcloud 
> This change was done IIRC to accomodate index level names that were the same as column names, but I would personally just regard that the same as duplicate column names (thus: not supported).

We introduced this to accommodate that use case specifically. See [ARROW-1754](https://issues.apache.org/jira/browse/ARROW-1754)

> Currently there is no strict mapping possible (or at least harder than it should be) between the pandas metadata and the actual field names in the schema

I don't understand this comment. Can you give an example of what isn't possible or is cumbersome here?

> because the order / number of entries in the pandas_metadata is not always the same as the fields when subsetting data

That's also true for some cases independent of how we deal with index names. For example `pq.read_parquet(..., columns=['b', 'a'])` if the order of b and a in the schema is `['a', 'b']`.

@jorisvandenbossche Can you give some examples to illustrate your points?
> > This change was done IIRC to accomodate index level names that were the same as column names, but I would personally just regard that the same as duplicate column names (thus: not supported).

> We introduced this to accommodate that use case specifically. See ARROW-1754

Yes, I know, that's why I mentioned it. But I would say to that issue: "pitty, won't fix, this is not supported" (or of course: raise an informative error message. The solution for the user can simply be to specify `preserve_index=False` in `Table.from_pandas`). For me this is very similar to writing dataframes with duplicate column names to parquet which is *also* not supported.

> > Currently there is no strict mapping possible (or at least harder than it should be) between the pandas metadata and the actual field names in the schema

> I don't understand this comment. Can you give an example of what isn't possible or is cumbersome here?

In https://github.com/apache/arrow/pull/1386, I needed to do the following to get the field name corresponding to a specific pandas metadata column entry:

```
index_columns = pandas_metadata['index_columns']
n_index_levels = len(index_columns)
n_columns = len(pandas_metadata['columns']) - n_index_levels

for i, col_meta in enumerate(pandas_metadata['columns']):

    raw_name = col_meta['name']
    if i >= n_columns:
        # index columns
        raw_name = index_columns[i - n_columns]
    if raw_name is None:
        raw_name = 'None'
        
    idx = schema.get_field_index(raw_name)
```

It might be that I am missing something and the above is overly complicated. But if you would have a `field_name` entry in the pandas metadata column entries, the above code could be simplified to:

```
for i, col_meta in enumerate(pandas_metadata['columns']):

    raw_name = col_meta['field_name']
    idx = schema.get_field_index(raw_name)
```

I would think such an operation (getting the field name corresponding to a pandas column name) is something rather common (it is also something that they will need in fastparquet), and it is of course not "impossible" (that was a bit strong stated), but just rather hard. 
On the other hand, in `pyarrow` itself you can easily make this into a helper function if you would need it in multiple places, so of course also not that much of a problem. But as somebody starting to play with pyarrow Tables from pandas dataframes, figuring this out (the relationship between the pandas names and field names, and coming up with the above code snippet) was not that straightforward from the beginning.

This is indeed not fully related to the index names discussion, because even if we would revert the "always using `__index_level_n__` pattern", you would need the above code anyhow. So it are two separate issues.
@cpcloud Another illustration of the possible difficulties with the mapping between the pandas metadata and the actual field names in the schema: currently this mapping is possible in `pyarrow` because it assumes that the index columns are the *last ones* inside the schema. 
However, this assumption breaks eg with files written by `fastparquet`, because they do not put index levels at the end, but in the beginning:

```
In [79]: df = pd.DataFrame({'a': [1, 2, 3], 'b': [.1, .2, .3], 'c':pd.date_range("2017-01-01", periods=3, tz='Europe/Brussels')}, index=pd.Index(['A', 'B', 'C'], name='index'))

In [80]: df
Out[80]: 
       a    b                         c
index                                  
A      1  0.1 2017-01-01 00:00:00+01:00
B      2  0.2 2017-01-02 00:00:00+01:00
C      3  0.3 2017-01-03 00:00:00+01:00

In [81]: fastparquet.write("__test_index_fastparquet-dev.parquet", df)
/home/joris/miniconda3/envs/pyarrow-dev/lib/python3.6/site-packages/fastparquet/writer.py:136: UserWarning: Coercing datetimes to UTC
  warnings.warn('Coercing datetimes to UTC')

In [82]: pq.read_table("__test_fastparquet-dev.parquet")
Out[82]: 
pyarrow.Table
index: string
a: int64
b: double
c: timestamp[us]
metadata
--------
{b'pandas': b'{"columns": [{"metadata": null, "name": "index", "numpy_type": "'
            b'object", "pandas_type": "unicode"}, {"metadata": null, "name": "'
            b'a", "numpy_type": "int64", "pandas_type": "int64"}, {"metadata":'
            b' null, "name": "b", "numpy_type": "float64", "pandas_type": "flo'
            b'at64"}, {"metadata": {"timezone": "Europe/Brussels"}, "name": "c'
            b'", "numpy_type": "datetime64[ns, Europe/Brussels]", "pandas_type'
            b'": "datetimetz"}], "index_columns": ["index"], "pandas_version":'
            b' "0.22.0.dev0+260.g5da3759"}'}

In [83]: pq.read_table("__test_fastparquet-dev.parquet").to_pandas()
...
ArrowNotImplementedError: No cast implemented from string to timestamp[ns, tz=Europe/Brussels]
```

Of course, this could (and maybe should) be fixed in `fastparquet` (they still have to update with regards to the index name change as well: https://github.com/dask/fastparquet/issues/251). And aonce fixed, this again will not be an impossible problem. However, this assumption was up to now not written in the pandas metadata specifications (what you are updating now), so they are not to blame. And to me it shows a bit the brittleness of this assumption.
I'm looking into this on the fastparquet / dask side.

> But if you would have a field_name entry in the pandas metadata column entries

Joris' suggestion seems sensible to me.
Whatever the decision for this discussion, this PR should close https://github.com/pandas-dev/pandas/issues/16391 (where Wes proposes an additional name entry for the index to disambiguate with column names)
@jorisvandenbossche @TomAugspurger Thanks for commenting. I agree adding `'field_name'` will be helpful and will allow writing index metadata that fastparquet can read as well. PR is in the works for arrow. I'll also update this PR with the changes.
Thanks.

Thanks. Once ARROW-1895 is ready I'll update fastparquet's metadata writer
to be compliant.

On Wed, Dec 6, 2017 at 1:50 PM, Phillip Cloud <notifications@github.com>
wrote:

> @jorisvandenbossche <https://github.com/jorisvandenbossche> @TomAugspurger
> <https://github.com/tomaugspurger> Thanks for commenting. I agree adding
> 'field_name' will be helpful and will allow writing index metadata that
> fastparquet can read as well. PR is in the works for arrow. I'll also
> update this PR with the changes.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/18201#issuecomment-349754480>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIloCpiOVkaff3vA-Q9JLE_2Cs5_wks5s9u_vgaJpZM4QYi4Q>
> .
>

Thanks! Happy to review when there is a PR.

Regarding my other point (the second bullet point here: https://github.com/pandas-dev/pandas/pull/18201#issuecomment-349359616) about always using `__index_level_n__` as the index column name in the schema. 
I was thinking a bit more about it, and there is maybe an alternative that does not bring back the bug fixed in [ARROW-1754](https://issues.apache.org/jira/browse/ARROW-1754), which is to use the  `__index_level_n__` only if the index name is None *or* already exists in the column names (both conditions should be easy to check). 

The reason that I argue for this is the following: as long as one uses `to_parquet`/`read_parquet` to roundtrip pandas dataframes, it does not really matter how the index columns are stored in the parquet file. But if one also wants to use those files in other systems, I can imagine that names like `__index_level_n__` can be rather annoying, certainly if your actual dataframe had a sensible index name. 
And we will by default write index columns in `to_parquet` (see in progress PR https://github.com/pandas-dev/pandas/pull/18629, until now only supported default integer range indexes), so this will become more common.

I understand that the 0.8.0 release is close, so that you might not want to reconsider this at this point. But I can certainly do a PR with the change to help out (I think it should be a relatively straightforward patch, just as https://github.com/apache/arrow/pull/1271 was also not that a big patch)

Here's the PR https://github.com/apache/arrow/pull/1397
This is unrelated to the changes here, but I noticed pyarrow uses "string" for the "pandas_type":

```python
>>> df = pd.DataFrame({"A": [1, 2]})
>>> sink = io.BytesIO()
>>> pq.write_metadata(pa.Table.from_pandas(df).schema, sink)
>>> json.loads(pq.read_metadata(sink).metadata[b'pandas'].decode('utf-8'))['column_indexes']
[{'metadata': None,
  'name': None,
  'numpy_type': 'object',
  'pandas_type': 'string'}]
```

Though the spec says just `'unicode'` or `'bytes'` for String type. Could we add `'string'` as an alias for unicode?
@TomAugspurger That only appears in the `'column_indexes'` metadata, because that doesn't go through arrow schemas, so that's a bug. This was added in between 0.7.1 and upcoming 0.8.0 so we can change this without any backward compatibility consequences.
@jorisvandenbossche @TomAugspurger Thanks for ironing this stuff out, it's very helpful to have a third and fourth pair of eyes on this code.
cc @martindurant
As I'm working on updating dask for this, I think allowing duplicates between the index names and columns as in https://issues.apache.org/jira/browse/ARROW-1754 necessitates some kind of ordering on `columns` in the pandas metadata.

Consider this (unlikely) dataframe

```python
df4 = pd.DataFrame({"__index_level_0__": [1, 2]},
                   index=pd.Index([0, 1], name='__index_level_0__'))
```

On https://github.com/apache/arrow/pull/1397 that produces this metadata:

```python
{'column_indexes': [{'field_name': None,
   'metadata': {'encoding': 'UTF-8'},
   'name': None,
   'numpy_type': 'object',
   'pandas_type': 'unicode'}],
 'columns': [{'field_name': '__index_level_0__',
   'metadata': None,
   'name': '__index_level_0__',
   'numpy_type': 'int64',
   'pandas_type': 'int64'},
  {'field_name': '__index_level_0__',
   'metadata': None,
   'name': '__index_level_0__',
   'numpy_type': 'int64',
   'pandas_type': 'int64'}],
 'index_columns': ['__index_level_0__'],
 'pandas_version': '0.21.0'}
```

So the two items in `columns` are identical, and it's impossible to disambiguate between the column and the index. This is admittedly a contrived example, but it'd be nice to have this be unambiguous.

I think we can either

1. Update the spec to require field ordering for the indexes
2. Update the spec for columns to include an optional flag like "is_index" (False by default)

I don't have a preference. Option 1 is less work for pyarrow, and gives me another motivation to update fastparquet to write the same metadata.

Other options are: 3) don't allow duplicates 4) make sure the field names are unique (and then the actual column names / index level names don't need to) eg by taking the first `__index_level_n__` one that does not yet exist in the column field names as index field name.

I actually don't mind for option 3 (seeing index levels as a special type of column, for me it is fine to extend the uniqueness constraint to index names as well). But if we want to allow it, I think I find option 4 the easiest.
@jorisvandenbossche can you give an example implementation of option 4? It seems like you're saying we should make the index field name generation depend on the names of the non index columns. If so, that seems far more complex than requiring index columns be last or adding an index flag. 
Will do, but just a quick check: does parquet allow columns with the same name? Or arrow Tables ?
So the example of @TomAugspurger above currenlty gives:

```
In [6]: df4 = pd.DataFrame({"__index_level_0__": [1, 2]},
   ...:                    index=pd.Index([0, 1], name='__index_level_0__'))
   ...:                    

In [7]: pa.Table.from_pandas(df4)
Out[7]: 
pyarrow.Table
__index_level_0__: int64
__index_level_0__: int64
metadata
--------
{b'pandas': b'{"index_columns": ["__index_level_0__"], "column_indexes": [{"na'
            b'me": null, "pandas_type": "string", "numpy_type": "object", "met'
            b'adata": null}], "columns": [{"name": "__index_level_0__", "field'
            b'_name": "__index_level_0__", "pandas_type": "int64", "numpy_type'
            b'": "int64", "metadata": null}, {"name": "__index_level_0__", "fi'
            b'eld_name": "__index_level_0__", "pandas_type": "int64", "numpy_t'
            b'ype": "int64", "metadata": null}], "pandas_version": "0.22.0.dev'
            b'0+260.g5da3759"}'}
```

so that results in a parquet file / arrow table with duplicate field names. Do we actually want to allow this being created?
> If so, that seems far more complex than requiring index columns be last or adding an index flag.

In my opinion, this is not more complex from a spec point of view. It is simply adding a requirement in the spec that the "field_names" should be unique. 
Of course it will add a bit complexity in the pyarrow code (see snippet below, but that is then the spec implementer's responsability), but the other options will also add complexity (eg if we use the guarantee of 'last items' in "columns" list, then fastparquet will need to update and keep the other format working for back compatibility)

For example, with current arrow master, and due to the assumption that index columns come last, `pyarrow` will read in a parquet file written by `fastparquet` wrongly (the resulting index names are wrong, the values are OK).

That said, I am not sure how important this specific corner case is that Tom raised. I don't know what arrow's view is on duplicate column names (see question above, it in any case seems allowed to create a Table with duplicate field names)

---

Here is a small snippet that would ensure unique index level field names. Of course it is more complex than the current `index_level_name = '__index_level_{:d}__'.format`; `list(map(index_level_name, range(len(index_levels))`, but I don't think it is overly complex:

```
In [52]: def get_index_column_names(n_index_levels, column_names):
    ...:     index_column_names = []
    ...:     i = 0
    ...:     for _ in range(n_index_levels):
    ...:         while True:
    ...:             index_level_name = '__index_level_{:d}__'.format(i)
    ...:             i += 1
    ...:             if index_level_name not in column_names:
    ...:                 index_column_names.append(index_level_name)
    ...:                 break
    ...:     return index_column_names

In [53]: get_index_column_names(2, ['col'])
Out[53]: ['__index_level_0__', '__index_level_1__']

In [54]: get_index_column_names(2, ['col', '__index_level_1__'])
Out[54]: ['__index_level_0__', '__index_level_2__']

```


Hm it does seem strange to me that pyarrow allows duplicate column names.
Let me investigate
One more thing on ordering, fastparquet and pyarrow are both implicitly assuming that the order in `columns` is the order of the columns in the final DataFrame. We should make that explicit in the metadata spec.

That's another reason I would prefer to do away with the `index_columns` array in favor of an `is_index` field in each item in `columns`. Then we can say

1. The (multi)index is all the items in `columns` where `is_index` is true. The order from implies outer- to inner-most level. A single item with `is_index` being true means a non-multiindex, and single-level multi-indexes are not allowed.
2. The columns are from all the items in `columns` where `is_index` is false, ordered left to right.

Sorry to be pedantic about this. In real-world use-cases, no one (probably) will name their columns `__index_level_0__`, and probably won't even have duplicates. But I figure that getting this right is possible to do, so we may as well.