Repository: pydata/xarray

Issue:
Dataset.to_netcdf with mixed int and str in data
Hi, 

The code below gives me a TypeError when use engine = ‘netcdf4’ or ‘scipy’ in to_netcdf() function, but works when use engine='h5netcdf' by automatically converting int to str. 

Any good idea to solve this issue? To save the data as their original dtype (not covert int to str)?

Code:
df = pd.DataFrame({'a' : ['x', 'y'], 'b': [1,2]})
da = xr.DataArray(df)
ds = xr.Dataset({'test' : da})

ds.to_netcdf('test.nc', engine='netcdf4')  #TypeError: expected bytes, int found
ds.to_netcdf('test.nc', engine='scipy')  #TypeError: expected bytes, int found
ds.to_netcdf('test.nc', engine='h5netcdf')  #works, but automatically covert int to str which is not good...








Discussion:
andersy005: # Before

```python
In [2]: data = np.array([["x", 1], ["y", 2]], dtype="object")

In [3]: xr.conventions._infer_dtype(data, 'test')
Out[3]: dtype('O')
```


# After

```python
In [2]: data = np.array([["x", 1], ["y", 2]], dtype="object")

In [3]: xr.conventions._infer_dtype(data, 'test')
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-3-addaab43c03a> in <module>
----> 1 xr.conventions._infer_dtype(data, 'test')

~/devel/pydata/xarray/xarray/conventions.py in _infer_dtype(array, name)
    142     native_dtypes = set(map(lambda x: type(x), array.flatten()))
    143     if len(native_dtypes) > 1:
--> 144         raise ValueError(
    145             "unable to infer dtype on variable {!r}; object array "
    146             "contains mixed native types: {}".format(

ValueError: unable to infer dtype on variable 'test'; object array contains mixed native types: str,int
```
pep8speaks: Hello @andersy005! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:

* In the file [`xarray/conventions.py`](https://github.com/pydata/xarray/blob/b3ecf0c3df817def11f04889478783e661aa79ca/xarray/conventions.py):

> [Line 142:61](https://github.com/pydata/xarray/blob/b3ecf0c3df817def11f04889478783e661aa79ca/xarray/conventions.py#L142): [F821](https://duckduckgo.com/?q=pep8%20F821) undefined name 'a'




andersy005: > Alternatives — not ideal ones — would be to wait until the main error is raised, or only test a subset of the values. 

I thought of taking a random sample from the array and checking the types on the sample only, but I wasn't so confident about how representative this sample would be and/or how to deal with misleading, skewed samples. If anyone has thoughts on this, please let me know. 
mathause: It took around 40 s for an array of 10**9 elements. That would be around 150 years of daily data (180\*360\*150\*365). I am not sure though how much sense it makes to have such a large array with object dtype. Also an array of this size is likely a dask array and there is already a performance warning on this. So I'd say go ahead. 

https://github.com/pydata/xarray/blob/68d3c340304ccc57cac569b22781e2b9c3dac913/xarray/conventions.py#L194-L197
andersy005: > Also an array of this size is likely a dask array and there is already a performance warning on this. So I'd say go ahead.

@mathause, just to make sure I am not misinterpreting your comment, is this a go ahead to sampling the array to determine the types? :) 
mathause: Yes, I'd say go ahead. (I just hope it's not too big of a performance hit for normal use cases.)
andersy005: > Yes, I'd say go ahead. (I just hope it's not too big of a performance hit for normal use cases.)

@mathause, I am noticing a performance hit even for the special use cases. Here's how I am doing the sampling

```python
sample_indices = np.random.choice(array.size, size=min(20, array.size), replace=False)
native_dtypes = set(np.vectorize(type, otypes=[object])(array.ravel()[sample_indices]))
```

and here's the code snippet I tested this on:


```python
In [1]: import xarray as xr, numpy as np

In [2]: x = np.asarray(list("abcdefghijklmnopqrstuvwxyz"), dtype="object")

In [3]: array = np.repeat(x, 5_000_000)

In [4]: array.size
Out[4]: 130000000

In [5]: array.dtype
Out[5]: dtype('O')
```

### Without sampling


```python
In [6]: %timeit xr.conventions._infer_dtype(array, "test")
7.63 s ± 515 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
```


### With sampling


```python
In [15]: %timeit xr.conventions._infer_dtype(array, "test")
8.31 s ± 395 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
```


mathause: No I wouldn't subsample. With normal use case I meant saving legitimate object arrays. I am not sure how often they occur in the wild.
github-actions[bot]: ## Unit Test Results
         6 files           6 suites   54m 27s :stopwatch:
16 229 tests 14 508 :heavy_check_mark: 1 721 :zzz: 0 :x:
90 570 runs  82 410 :heavy_check_mark: 8 160 :zzz: 0 :x:

Results for commit 2107949c.

max-sixty: > No I wouldn't subsample. With normal use case I meant saving legitimate object arrays. I am not sure how often they occur in the wild.

I think this is most likely for lots of strings which aren't using a numpy string type.

@andersy005 to the extent the perf difference is 7 seconds vs 8 seconds, I would definitely vote to merge. The case I was originally concerned about is that something suddenly takes 1000 seconds. If we're confident that that's not the case, I would also vote to merge...

Patch:
diff --git a/xarray/conventions.py b/xarray/conventions.py
index 75f816e6..8c7d6be2 100644
--- a/xarray/conventions.py
+++ b/xarray/conventions.py
@@ -52,16 +52,32 @@ def _var_as_tuple(var: Variable) -> T_VarTuple:
     return var.dims, var.data, var.attrs.copy(), var.encoding.copy()
 
 
-def _infer_dtype(array, name: T_Name = None) -> np.dtype:
-    """Given an object array with no missing values, infer its dtype from its
-    first element
-    """
+def _infer_dtype(array, name=None):
+    """Given an object array with no missing values, infer its dtype from all elements."""
     if array.dtype.kind != "O":
         raise TypeError("infer_type must be called on a dtype=object array")
 
     if array.size == 0:
         return np.dtype(float)
 
+    native_dtypes = set(np.vectorize(type, otypes=[object])(array.ravel()))
+    if len(native_dtypes) > 1 and native_dtypes != {bytes, str}:
+        raise ValueError(
+            "unable to infer dtype on variable {!r}; object array "
+            "contains mixed native types: {}".format(
+                name, ", ".join(x.__name__ for x in native_dtypes)
+            )
+        )
+
+    native_dtypes = set(np.vectorize(type, otypes=[object])(array.ravel()))
+    if len(native_dtypes) > 1 and native_dtypes != {bytes, str}:
+        raise ValueError(
+            "unable to infer dtype on variable {!r}; object array "
+            "contains mixed native types: {}".format(
+                name, ", ".join(x.__name__ for x in native_dtypes)
+            )
+        )
+
     element = array[(0,) * array.ndim]
     # We use the base types to avoid subclasses of bytes and str (which might
     # not play nice with e.g. hdf5 datatypes), such as those from numpy


Test patch:
diff --git a/xarray/tests/test_conventions.py b/xarray/tests/test_conventions.py
index d6d1303a..be6e949e 100644
--- a/xarray/tests/test_conventions.py
+++ b/xarray/tests/test_conventions.py
@@ -495,6 +495,18 @@ class TestCFEncodedDataStore(CFEncodedBase):
         pass
 
 
+@pytest.mark.parametrize(
+    "data",
+    [
+        np.array([["ab", "cdef", b"X"], [1, 2, "c"]], dtype=object),
+        np.array([["x", 1], ["y", 2]], dtype="object"),
+    ],
+)
+def test_infer_dtype_error_on_mixed_types(data):
+    with pytest.raises(ValueError, match="unable to infer dtype on variable"):
+        conventions._infer_dtype(data, "test")
+
+
 class TestDecodeCFVariableWithArrayUnits:
     def test_decode_cf_variable_with_array_units(self) -> None:
         v = Variable(["t"], [1, 2, 3], {"units": np.array(["foobar"], dtype=object)})