{"action":"created","issue":{"url":"https://api.github.com/repos/pydata/xarray/issues/988","repository_url":"https://api.github.com/repos/pydata/xarray","labels_url":"https://api.github.com/repos/pydata/xarray/issues/988/labels{/name}","comments_url":"https://api.github.com/repos/pydata/xarray/issues/988/comments","events_url":"https://api.github.com/repos/pydata/xarray/issues/988/events","html_url":"https://github.com/pydata/xarray/issues/988","id":173612265,"number":988,"title":"Hooks for custom attribute handling in xarray operations","user":{"login":"shoyer","id":1217238,"avatar_url":"https://avatars.githubusercontent.com/u/1217238?v=3","gravatar_id":"","url":"https://api.github.com/users/shoyer","html_url":"https://github.com/shoyer","followers_url":"https://api.github.com/users/shoyer/followers","following_url":"https://api.github.com/users/shoyer/following{/other_user}","gists_url":"https://api.github.com/users/shoyer/gists{/gist_id}","starred_url":"https://api.github.com/users/shoyer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shoyer/subscriptions","organizations_url":"https://api.github.com/users/shoyer/orgs","repos_url":"https://api.github.com/users/shoyer/repos","events_url":"https://api.github.com/users/shoyer/events{/privacy}","received_events_url":"https://api.github.com/users/shoyer/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2016-08-27T19:48:22Z","updated_at":"2016-08-28T01:15:52Z","closed_at":null,"body":"Over in #964, I am working on a rewrite/unification of the guts of xarray's logic for computation with labelled data. The goal is to get all of xarray's internal logic for working with labelled data going through a minimal set of flexible functions which we can also expose as part of the API.\r\n\r\nBecause we will finally have all (or at least nearly all) xarray operations using the same code path, I think it will also finally become feasible to open up hooks allowing extensions how xarray handles metadata.\r\n\r\nTwo obvious use cases here are units (#525) and automatic maintenance of metadata (e.g., [`cell_methods`](https://github.com/pydata/xarray/issues/987#issuecomment-242912131) or [`history`](#826) fields). Both of these are out of scope for xarray itself, mostly because the specific logic tends to be domain specific. This could also subsume options like the existing `keep_attrs` on many operations.\r\n\r\nI like the idea of supporting something like NumPy's [`__array_wrap__`](http://docs.scipy.org/doc/numpy-1.11.0/reference/arrays.classes.html#numpy.class.__array_wrap__) to allow third-party code to finalize xarray objects in some way before they are returned. However, it's not obvious to me what the right design is.\r\n\r\n- Should we lookup a custom attribute on subclasses like `__array_wrap__` (or `__numpy_ufunc__`) in NumPy, or should we have a system (e.g., unilaterally or with a context manager and `xarray.set_options`) for registering hooks that are then checked on *all* xarray objects? I am inclined toward the later, even though it's a little slower, just because it will be simpler and easier to get right\r\n- Should these methods be able to control the full result objects, or only set `attrs` and/or `name`?\r\n- To be useful, do we need to allow extensions to take control of the full operation, to support things like automatic unit conversion? This would suggest something closing to `__numpy_ufunc__`, which is a little more ambitious than what I had in mind here.\r\n\r\nFeedback would be greatly appreciated.\r\n\r\nCC @darothen @rabernat @jhamman @pwolfram"},"comment":{"url":"https://api.github.com/repos/pydata/xarray/issues/comments/242950070","html_url":"https://github.com/pydata/xarray/issues/988#issuecomment-242950070","issue_url":"https://api.github.com/repos/pydata/xarray/issues/988","id":242950070,"user":{"login":"shoyer","id":1217238,"avatar_url":"https://avatars.githubusercontent.com/u/1217238?v=3","gravatar_id":"","url":"https://api.github.com/users/shoyer","html_url":"https://github.com/shoyer","followers_url":"https://api.github.com/users/shoyer/followers","following_url":"https://api.github.com/users/shoyer/following{/other_user}","gists_url":"https://api.github.com/users/shoyer/gists{/gist_id}","starred_url":"https://api.github.com/users/shoyer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shoyer/subscriptions","organizations_url":"https://api.github.com/users/shoyer/orgs","repos_url":"https://api.github.com/users/shoyer/repos","events_url":"https://api.github.com/users/shoyer/events{/privacy}","received_events_url":"https://api.github.com/users/shoyer/received_events","type":"User","site_admin":false},"created_at":"2016-08-28T01:15:52Z","updated_at":"2016-08-28T01:15:52Z","body":"Let me give a concrete example of what I think this interface should look like.\r\n\r\nTo implement units:\r\n```python\r\nfrom typing import List, Optional  # optional Python 3.5 type annotations\r\n\r\n@xarray.register_ufunc_variables_attrs_handler\r\ndef propagate_units(results: List[xarray.Variable],\r\n                    context: xarray.UFuncContext) -> Optional[List[dict]]:\r\n    if context.func.__name__ in ['add', 'sub']:\r\n        units_set = set(getattr(arg, 'attrs', {}).get('units') for arg in context.args)\r\n        if len(units_set) > 1:\r\n            raise ValueError('not all input units the same: %r' % units_set)\r\n        units, = units_set\r\n        return [{'units': units}]\r\n    else:\r\n        return [] * len(results)\r\n        # or equivalently, don't return anything at all\r\n```\r\nOr to (partially) handle `cell_methods`:\r\n```python\r\n@xarray.register_ufunc_variables_attrs_handler\r\ndef add_cell_methods(results, context):\r\n    if context.func.__name__ in ['mean', 'median', 'sum', 'min', 'max', 'std']):\r\n        dims = set(context.args[0].dims) - set(results[0].dims)\r\n        cell_methods = ': '.join(dims) + ': ' + context.func.__name__\r\n        return [{'cell_methods': cell_methods})\r\n```\r\nOr to implement `keep_attrs=True` if a function only has one input:\r\n```python\r\n@xarray.register_ufunc_variables_attrs_handler\r\ndef always_keep_attrs(results, context):\r\n    if len(context.args) == 1:\r\n        return [context.args[0].attrs] * len(result)\r\n```\r\n\r\nEvery time xarray does an operation, we would call all of these registered `ufunc_variables_attrs_handlers` to get list of attributes to add to result `Variable`. `attrs` on the resulting object (or objects if the ufunc has multiple outputs) would be accumulated by calling the handlers in arbitrary order and merging the resulting dicts. Repeated keys in the `attrs` dicts returned by different handlers would result in an error.\r\n\r\n`xarray.UFuncContext` itself would be a simple struct-like class with a few attributes:\r\n- `func`: the function being applied. Typically from NumPy or dask.array, but also could be an arbitrarily callable if a user calls `xarray.apply_ufunc` directly.\r\n- `args`: positional arguments passed into the function. Possibly xarray `Variable` objects, `numpy.ndarray` or scalars.\r\n- `kwargs`: additional dict of keyword arguments.\r\n\r\nSimilarly, we would have `register_ufunc_dataset_attrs_handler` for updating `Dataset` attrs.\r\n\r\nThe downside of this approach is that unlike the way NumPy handles things, this doesn't handle conflicting implementations well. If you try to use two different libraries that register their own global attribute handlers instead of using the context manager (e.g., two different units implementations), things will break, even if the unrelated code paths do not touch.\r\n\r\nSo alternatively to using the registration system, we could support/encourage using a context manager, e.g.,\r\n```python\r\nwith xarray.ufunc_variables_attrs_handlers([always_keep_attrs, add_cell_methods]):\r\n     # either augment or ignore other attrs handlers, possibly depending\r\n     # on the choice of a keyword argument to ufunc_variables_attrs_handlers\r\n     result = ds.mean()\r\n```\r\nIt's kind of verbose, but certainly useful for libraries that want to be cautious about breaking other code. In general, it's poor behavior for libraries to unilaterally change unrelated code without an explicit opt-in. So perhaps the best approach is to encourage users to *always* use a context manager, e.g.,\r\n```python\r\nimport contextlib\r\n\r\n@contextlib.contextmanager\r\ndef my_attrs_context():\r\n    with xarray.ufunc_variables_attrs_handlers(\r\n            [always_keep_attrs, add_cell_methods, ...]):\r\n        yield\r\n\r\nwith my_attrs_context():\r\n    result = ds.mean() - 0.5 * (ds.max() - ds.min())\r\n```\r\n\r\nSo maybe a subclass based implementation (with a custom attribute like `__xarray_attrs_handler__`) is the cleanest way to handle this, after all."}}
{"actor":{"display_login":"shoyer"}}