COMPAT: Objects construction compat with xarray.Dataset

[max-sixty]

• closes Supplying an xarray Dataset to DataFrame constructor breaks #12353
• tests added
• tests passed
• passes git diff upstream/master | flake8 --diff
• whatsnew entry

I'm changed the dict / mapping issue, but I can't get this part working (so the tests fail at the moment):

But pd.Series(dict(xr.Dataset({'a': 2, 'b': 3}))) doesn't work either, because the values in the dict are 0-th dimension arrays, and Series doesn't unpack them (list(dict(xr.Dataset({'a': 2, 'b': 3})).values())[0] for the full code)
#12353 (comment)

Is that a reasonable goal? Where would be a good place to do the conversion?

[max-sixty]

This was unintentional - PyCharm - although it is nicer now...

[jreback]

@MaximilianR since you are fixing this, thanks!

can you incoporate anything from my original PR that is needed: #12356

[max-sixty]

@jreback Ready to go, less a what's new note and removing a commented out test (this is why the build fails).

Apologies for mixing formatting & functional improvements.

Needs a look through from you, in particular the to_dataframe check.

[max-sixty]

Because of the issues with 0-d arrays discussed in the issue

[jreback]

@MaximilianR if you'd like to rebase/update

[max-sixty]

Rebased.
@jreback, awaiting your review

[jreback]

put things back in this file as these are not substantive changes (and things getting changed elsewhere)

[jreback]

you have some setting on your editor which just messes with existing (correct) linting / formatting. Pls turn that off. Makes it much harder to see what is changing.

Overall I think this approach is nice. .to_dataframe() is a good approach / very duck-like.

[max-sixty]

I've been remiss about the final fixes here. Will need a couple of weeks on other commitments but I will get to this

[shoyer]

I would slightly rather skip this special check. In theory, to_dataframe might return something else.

I don't think it will be that much slower to use the generic dict path, though we then run into the issue that DataFrame._init_dict does a special check for OrderedDict when deciding whether or not to order the keys. Sadly there's no way in check whether an arbitrary Mapping type has intentionally ordered keys or not (Python-ideas discussed adding collections.abc.Ordered but I don't think it was implemented).

[max-sixty]

The broader question is whether we want objects to be able to define their own conversion to a DataFrame - I think the main options are:

1. Everything is treated as a dict
2. We build the code within the DataFrame constructor - i.e. that if statement checks for DataSet (and anything else we want to check)
3. There is some duck-like method such as to_dataframe that classes can define themselves

Thoughts?

[shoyer]

I'm happy with a duck-like method, but we should probably call it something very explicit like _to_pandas_dataframe_. This would be useful for a lot of other projects, not just xarray.

[jreback]

@MaximilianR ?

[max-sixty]

Apologies! I will do soon. Please don't close.

[jreback]

@MaximilianR want to rebase?

I would be onboard with allowing a special method __dataframe__ which is pretty clear IMHO.

[shoyer]

I would be onboard with allowing a special method dataframe which is pretty clear IMHO.

At this point there are a number of alternative "dataframe" objects (e.g., dask, spark), many of which probably want to be coercible to pandas, so I think the special method should have pandas in the name to clarify. Hence my suggestion for _to_pandas_dataframe_.

[jreback]

status?

[jreback]

can you rebase / update

[max-sixty]

Yes, thanks for your patience.

What do we want to name the method? __dataframe__? _to_dataframe?

[jreback]

__dataframe__. I don't think it matters that others have dataframes. pandas is the standard in python, just as numpy is for arrays.

[shoyer]

I feel pretty strongly that the name should not just be __dataframe__ and we should have pandas in it somewhere.

If we were starting over with NumPy, the name probably wouldn't be __array__, either.

[jreback]

I don't remember If I suggested the special method, but what is wrong with .to_dataframe()? (or .to_pandas()?

[shoyer]

I don't remember If I suggested the special method, but what is wrong with .to_dataframe()? (or .to_pandas()?

The main reason is that it's totally conceivable that some application might use these for something else, e.g., to_dataframe() might make a spark dataframe instead. In fact, xarray sometimes uses to_pandas() to create a pandas.Series (if the array is 1D). The advantage of looking for a special method with a name like _to_pandas_dataframe_ is that there is almost no chance that it will be used incorrectly.

[jreback]

@shoyer

Is this supposed to work?

In [30]: x = DataFrame({'A': [1, 2, 3], 'B': [3., 4., 5.]}).to_xarray()

In [31]: x
Out[31]: 
<xarray.Dataset>
Dimensions:  (index: 3)
Coordinates:
  * index    (index) int64 0 1 2
Data variables:
    A        (index) int64 1 2 3
    B        (index) float64 3.0 4.0 5.0

In [32]: np.array(x)
Out[32]: 
array(['index', 'A', 'B'], 
      dtype='<U5')

[shoyer]

@jreback xarray.Dataset is not meant to be convertable to a NumPy array, so it doesn't define __array__. But it does subclass collections.Mapping, so NumPy apparently manages to convert it by iterating over the keys, like this class:

class WrapMapping(collections.Mapping):
  def __init__(self, mapping):
    self._map = mapping
  def __getitem__(self, key):
    return self._map[key]
  def __iter__(self):
    return iter(self._map)
  def __len__(self):
    return len(self._map)

>>> np.array(WrapMapping({'a': 1, 'b': 2}))
array(['b', 'a'], 
      dtype='|S1')

This is totally useless though, so I'll make a note to implement a __array__ for Dataset that just raises TypeError with a helpful error message.

[jreback]

@shoyer gotcha, ok, then I guess __pandas_dataframe__ is prob reasonable for external converters.

[jreback]

@MaximilianR rebase?

[jreback]

can you rebase and update?

[jreback]

closing as stale