issue_comments

Thanks, I'll try to use the github version.

Cheers

@guziy This should be fixed by #1133, which will be part of the next release.

Hi:

Here I have an example that have worked until recently...

https://github.com/guziy/PyNotebooks/blob/master/xarray/test_grouping.ipynb

Of course there is a resample function, but sometimes you want grouping...

Are there any plans to add a function as a key to the groupby function?

Cheers

Great, safe_cast_to_index works nicely (it passes my test). I added the change to the existing PR.

Do we need to add more groupby tests to make sure the solution is safe for other cases (e.g. other data types)?

Thanks for looking into this!

Based on how factorize works (with specialized handling for pandas dtypes), I think the most robust behavior would be to pass in a pandas.Index. For example, this will work better if someone uses a pandas.PeriodIndex. So I would suggest wrapping arrays with safe_cast_to_index before passing them to pd.factorize.

I'm not sure if we want a .view attribute on DataArrays, but in any case it's not clear that would even fix the issue here -- pandas probably needs to a coerce to numpy arrays internally in factorize eventually anyways.

OK, here is the minimal example:

```python import xarray as xr import pandas as pd

def test_groupby_da_datetime(): """groupby with a DataArray of dtype datetime""" # create test data times = pd.date_range('2000-01-01', periods=4) foo = xr.DataArray([1,2,3,4], coords=dict(time=times), dims='time')

# create test index
dd = times.to_datetime()
reference_dates = [dd[0], dd[2]]
labels = reference_dates[0:1]*2 + reference_dates[1:2]*2
ind = xr.DataArray(labels, coords=dict(time=times), dims='time', name='reference_date')

# group foo by ind
g = foo.groupby(ind)

# check result
actual = g.sum(dim='time')
expected = xr.DataArray([3,7], coords=dict(reference_date=reference_dates), dims='reference_date')
assert actual.to_dataset(name='foo').equals(expected.to_dataset(name='foo'))

```

Making that, I found out that the problem only occurs when the DataArray used with groupby has dtype=datetime64[ns].

The problem is that we effectively feed the DataArray to pd.factorize and that goes well for most data types: Pandas checks with the function needs_i8_conversion whether it can factorize the DataArray and decides YES for our datetime64[ns]. But then in pd.factorize it fails because it tries to access DataArray.view to convert to int64.

So as I see it there are three possible solutions to this: 1. Make Pandas' pd.factorize handle our datetime DataArrays better, 2. Add an attribute .view to DataArrays, or 3. Use the solution in the above PR, which means feeding only the NumPy .values to pd.factorize.

This looks like a plausible fix, but what would be really helpful is a minimal, complete example that triggers the error. That should help clarify the issue and at the least, we will need that for a test case in your pull request.