issue_comments

Perhaps we should have array_to_list: bool instead. If False, we just preserve the underlying array type.

Then the user could do ds.as_numpy().to_dict(array_to_list=False) to always get numpy arrays as #7739

The PR #7739 is available for review. @jhamman @dcherian would be my choices. i think this is pretty straight forward. I suppose the name of the kwarg being numpy_data is debatable. I based this on the discussion of numpy vs tolist above, preferring the former but acknowledging the comment that a package name as an arg is odd. Could do as_numpy or something slightly different.

@jmccreight - I don't think there is any specific reason this didn't get done. Still open to contributions here if you are interested.

I'd be interested in reviving this, this is exactly what I want to achieve. It's not clear if there was some reason this never went ahead. I looked around but didnt find anything. LMK if it there's some reason not to pursue it. THanks

In order to maintain a list of currently relevant issues, we mark issues as stale after a period of inactivity

If this issue remains relevant, please comment here or remove the stale label; otherwise it will be marked as closed automatically

@nicain myself, @jhamman and/or @MaximilianR are good choices for reviewers, thanks!

Sweet, this is exactly what I am used to! Ill give a shot to writing a few tests later today, and make a PR. Can you suggest some core devs I can ping for a PR review? Ill ask @chrisbarber (we work together) to review, but I'd like more eyeballs on it.

We need to write a contributing guide!

But in brief: - We use the NumPy docstring convention: https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt - For unit tests, we use pytest, with helper functions/methods for verifying comparisons between objects. Take a look at the existing tests for examples to emulate: https://github.com/pydata/xarray/blob/25d1855e737444c156f50d1f37a67d9674a8bac5/xarray/tests/test_dataset.py#L2402

See also: https://github.com/pydata/xarray/pull/1485/files

@chrisbarber

Here is my example implementation: https://github.com/nicain/xarray/blob/fa86e3d38ebf4e641cafd963a5b69a77539b931d/xarray/core/dataarray.py#L1388

@shoyer Can you point me to where to edit documentation and unit test best practices?

@nicain, for sure. Probably best for the API's sake to stick to the simplicity of a flag.

@chrisbarber Maybe a method to_byte_dict that (in implementation) takes the output from numpy=True and walks the dict with tobytes?

Could have a callable serializer kwarg that defaults to np.ndarray.tolist. I have a use case where I would pass in np.ndarray.tobytes for this. But then again, I could just use numpy=True or tolist=False and then walk the dict myself.

I'll let others chime in on the API design here but yes that sounds pretty reasonable, too.

Yeah, my thoughts exactly. I am almost done with this in my fork... numpy might be confusing for those who will see the module name when they look at it. How does a kwarg:

Python def to_dict(self, tolist=True):

sound?

We could add a keyword argument to control this, e.g., numpy=True to indicate that data should be kept as NumPy arrays.