issue_comments
24 rows where issue = 173612265 sorted by updated_at descending
This data as json, CSV (advanced)
Suggested facets: reactions, created_at (date), updated_at (date)
issue 1
- Hooks for custom attribute handling in xarray operations · 24 ✖
id | html_url | issue_url | node_id | user | created_at | updated_at ▲ | author_association | body | reactions | performed_via_github_app | issue |
---|---|---|---|---|---|---|---|---|---|---|---|
482925611 | https://github.com/pydata/xarray/issues/988#issuecomment-482925611 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDQ4MjkyNTYxMQ== | eric-wieser 425260 | 2019-04-14T07:06:29Z | 2019-04-14T07:06:41Z | NONE |
Looks like that ship has sailed, PEP-472 has been rejected |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
413732471 | https://github.com/pydata/xarray/issues/988#issuecomment-413732471 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDQxMzczMjQ3MQ== | shoyer 1217238 | 2018-08-17T01:39:30Z | 2018-08-17T01:39:30Z | MEMBER |
Yep, this is pretty much what I was thinking of.
The virtue of this approach vs setting an global "attribute handler" (as suggested here) is that everything is controlled locally. For example, suppose people want to plug in two separate unit systems into xarray (e.g., pint and unyt). If the unit handling is determined by the specific arrays, then libraries relying on both approaches internally can happily co-exist and even call each other. In principle, this could be done safely with global handlers if you always know exactly when to switch back and forth, but that requires explicitly switching on handlers for even basic arithmetic. I doubt most users are going to bother, which is going to make using multiple tools that make use of this feature really hard. The other big advantage is that you only have to write the bulk of the unit system once, e.g., to define operations on NumPy arrays.
Rather than struggling to keep We could still do some work on the xarray side to make this easy to use. Specifically:
- The |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
413723632 | https://github.com/pydata/xarray/issues/988#issuecomment-413723632 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDQxMzcyMzYzMg== | dopplershift 221526 | 2018-08-17T00:33:47Z | 2018-08-17T00:33:47Z | CONTRIBUTOR | I see your argument, but here's my problem. In this future where things work (assuming that NEP is accepted), and I want distributed computing with dask, units, and xarray, I have: xarray wrapping a pint array wrapping a dask array. I like composition, but that level of wrapping...feels wrong to me for some reason. Is there some elegance I'm missing here? (Other than array-like things playing together.) And then I still need hooks in xarray so that when pint does a calculation, it can update the metadata in xarray; so it feels like we're back here anyway. |
{ "total_count": 1, "+1": 1, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
413409482 | https://github.com/pydata/xarray/issues/988#issuecomment-413409482 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDQxMzQwOTQ4Mg== | shoyer 1217238 | 2018-08-16T03:02:53Z | 2018-08-16T03:02:53Z | MEMBER |
I think these overloads are a much more maintainable way to add features like unit handling into xarray, as outlined in our development roadmap. It's not a complete system for overloading attribute handling in |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
413360749 | https://github.com/pydata/xarray/issues/988#issuecomment-413360749 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDQxMzM2MDc0OQ== | dopplershift 221526 | 2018-08-15T22:36:21Z | 2018-08-15T22:36:21Z | CONTRIBUTOR | @shoyer I know elsewhere you said you weren't sure about this idea any more, but personally I'd like to push forward on this idea. Do you have problems with this approach we need to resolve? Any chance you have some preliminary code? I think this is the right way to solve the unit issue in XArray, since at it's core unit handling is mostly a metadata operation. |
{ "total_count": 2, "+1": 2, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
316393986 | https://github.com/pydata/xarray/issues/988#issuecomment-316393986 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDMxNjM5Mzk4Ng== | peterkamatej 11941546 | 2017-07-19T13:51:09Z | 2017-07-19T13:51:09Z | NONE | Hi, I would really appreciate having some simple way to store some persisting attributes in xarray objects. One typical use case in my interactive work is that I have functions that have a DataArray or Dataset object as their only input, they perform some processing on the data and return an updated Dataset. Some additional information needed for this processing, such as an integer identifier of the experiment from which the data come from, are stored as attributes, so that when I have multiple instances of the same kind of objects, I will not mess up data from different experiments together. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
284314734 | https://github.com/pydata/xarray/issues/988#issuecomment-284314734 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI4NDMxNDczNA== | shoyer 1217238 | 2017-03-06T06:39:38Z | 2017-03-06T06:39:38Z | MEMBER | There's some chance that In general, this is a pretty tough design problem, which explains why it hasn't been solved yet :). But I was pretty happy with the way our Speaking of PEP 472, if someone has energy to push on that, it would be really awesome to see that happen. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
283531258 | https://github.com/pydata/xarray/issues/988#issuecomment-283531258 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI4MzUzMTI1OA== | gerritholl 500246 | 2017-03-02T01:51:08Z | 2017-03-02T01:51:08Z | CONTRIBUTOR | We do often deal with those in my line of work as well, I just happen not to right now. But time is the one thing that already carries units, doesn't it? One can convert between various |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
283524084 | https://github.com/pydata/xarray/issues/988#issuecomment-283524084 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI4MzUyNDA4NA== | lamorton 23484003 | 2017-03-02T01:09:44Z | 2017-03-02T01:09:44Z | NONE | @gerritholl In my line of work we often deal with 2+1 or 3+1 dimensional datasets (space + time). I have been bitten when I expected space in meters, but it was in centimeters, or time in seconds but it was in milliseconds. Also, I would like to improve the plotting functionality so that publication-quality plots can be made directly by automatically including units in the axis labels (and while I'm wishing for a pony, there could be pretty-printing versions of coordinate names (ie, LaTeX symbols or something)). |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
283515941 | https://github.com/pydata/xarray/issues/988#issuecomment-283515941 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI4MzUxNTk0MQ== | gerritholl 500246 | 2017-03-02T00:22:18Z | 2017-03-02T00:22:18Z | CONTRIBUTOR | Good point. I didn't think of that; my coordinates happen to be either time or unitless, I think. How common is it though that the full power of a unit library is needed for coordinates? I suppose it arises with indexing, i.e. the ability to write When it's a bit more polished I intend to publish it somewhere, but currently several things are missing ( |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
283492897 | https://github.com/pydata/xarray/issues/988#issuecomment-283492897 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI4MzQ5Mjg5Nw== | lamorton 23484003 | 2017-03-01T22:32:24Z | 2017-03-01T22:32:24Z | NONE | @gerritholl Interesting! The difficulty I am seeing with this approach is that the units apply only to the main data array, and not the coordinates. In a scientific application, the coordinates are generally physical quantities with units as well. If we want xarray with units to be really useful for scientific computation, we need to have the coordinate arrays be unitful 'quantities' too, rather than tacking the units on as an attribute of xarray.DataArray. I tinkered with making the 'units' attribute into a dictionary, with units for each coordinate (and for the data) as key-value pairs, but it is very cumbersome and goes against my philosophy (for instance, extracting a coordinate from a DataArray leaves it without units). |
{ "total_count": 1, "+1": 1, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
282273509 | https://github.com/pydata/xarray/issues/988#issuecomment-282273509 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI4MjI3MzUwOQ== | gerritholl 500246 | 2017-02-24T11:49:42Z | 2017-02-24T11:49:42Z | CONTRIBUTOR | I wrote a small recipe that appears to contain basic functionality I'm looking for. There's plenty of caveats but it could be a start, if such an approach is deemed desirable at all. ``` from common import ureg # or ureg = pint.UnitRegistry() import operator import xarray class UnitsAwareDataArray(xarray.DataArray): """Like xarray.DataArray, but transfers units """
for tp in ("add", "sub", "mul", "matmul", "truediv", "floordiv", "mod", "divmod"): meth = "{:s}".format(tp) def func(self, other, meth=meth, tp=tp): x = getattr(super(UnitsAwareDataArray, self), meth)(other) return self._apply_binary_op_to_units(getattr(operator, tp), other, x) func.name = meth print(func, id(func)) setattr(UnitsAwareDataArray, meth, func) del func ``` |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
282082423 | https://github.com/pydata/xarray/issues/988#issuecomment-282082423 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI4MjA4MjQyMw== | shoyer 1217238 | 2017-02-23T18:44:43Z | 2017-02-23T18:44:43Z | MEMBER |
Definitely not, I'm afraid. It's gone back and forth several times on master but hasn't landed yet. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
282081462 | https://github.com/pydata/xarray/issues/988#issuecomment-282081462 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI4MjA4MTQ2Mg== | gerritholl 500246 | 2017-02-23T18:41:19Z | 2017-02-23T18:41:19Z | CONTRIBUTOR | Is it not? The documentation says it's new in numpy 1.11 and we're at 1.12 now. I tried to make a small units-aware subclass of ``` class UnitsAwareDataArray(xarray.DataArray): """Like xarray.DataArray, but transfers units """
``` |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
282075293 | https://github.com/pydata/xarray/issues/988#issuecomment-282075293 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI4MjA3NTI5Mw== | shoyer 1217238 | 2017-02-23T18:18:36Z | 2017-02-23T18:18:36Z | MEMBER |
We currently have the binary arithmetic logic in |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
282070342 | https://github.com/pydata/xarray/issues/988#issuecomment-282070342 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI4MjA3MDM0Mg== | gerritholl 500246 | 2017-02-23T18:00:32Z | 2017-02-23T18:00:46Z | CONTRIBUTOR | Apparently |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
282063849 | https://github.com/pydata/xarray/issues/988#issuecomment-282063849 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI4MjA2Mzg0OQ== | gerritholl 500246 | 2017-02-23T17:37:18Z | 2017-02-23T17:37:18Z | CONTRIBUTOR | I would say using the ``` ureg is a pint unit registryy = a/b y.attrs["units"] = ureg(a.attrs["units"]) / ureg(b.attrs["units"]) ``` which if I understand the codebase correctly could be added to |
{ "total_count": 1, "+1": 1, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
280574774 | https://github.com/pydata/xarray/issues/988#issuecomment-280574774 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI4MDU3NDc3NA== | shoyer 1217238 | 2017-02-17T07:25:52Z | 2017-02-17T07:25:52Z | MEMBER | Some related discussion that may be of interest to participants here is going on over in #1271. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
243980885 | https://github.com/pydata/xarray/issues/988#issuecomment-243980885 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI0Mzk4MDg4NQ== | shoyer 1217238 | 2016-09-01T05:37:08Z | 2016-09-01T05:37:08Z | MEMBER | So I guess I guess we can also start with the attrs only hooks for now and add the others later if/as necessary. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
243974692 | https://github.com/pydata/xarray/issues/988#issuecomment-243974692 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI0Mzk3NDY5Mg== | pwolfram 4295853 | 2016-09-01T04:44:21Z | 2016-09-01T04:44:21Z | CONTRIBUTOR | +1 on requiring attribute handlers to be registered at a single location because this will make for cleaner code long term. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
243952679 | https://github.com/pydata/xarray/issues/988#issuecomment-243952679 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI0Mzk1MjY3OQ== | robintw 296686 | 2016-09-01T01:41:51Z | 2016-09-01T01:41:51Z | CONTRIBUTOR | I also prefer an approach that doesn't use context managers: I agree with @darothen's comments about the issues with their use. Regardless of the exact implementation, I am strongly in favour of this functionality being added to xarray - I can already think of a number of very useful ways I could use it! |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
243289800 | https://github.com/pydata/xarray/issues/988#issuecomment-243289800 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI0MzI4OTgwMA== | shoyer 1217238 | 2016-08-29T23:35:35Z | 2016-08-29T23:35:35Z | MEMBER | I agree that end users are likely to set this flag unilaterally, especially for interactive use. That's fine. This could even be OK in a higher level library, though I would encourage requiring an explicit opt in application code. One thing to consider is whether to allow multiple attribute handlers to be registered simultaneously or not. I kind of like a set_options interface that requires all handlers to be registered at once (as opposed to adding handlers incrementally ), because that ensures conflicts cannot arise inadvertantly. Either way, I don't think the performance penalty here would be significant in most cases, given how much of Python's dynamic nature xarray already uses. |
{ "total_count": 1, "+1": 1, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
243124532 | https://github.com/pydata/xarray/issues/988#issuecomment-243124532 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI0MzEyNDUzMg== | darothen 4992424 | 2016-08-29T13:32:11Z | 2016-08-29T13:32:11Z | NONE | I definitely see the logic with regards to encouraging users to use a context manager, and from the perspective of someone building a third-party library on top of xarray it would be fine. However, I think that from the perspective of an end-user (for example, a scientist) crunching numbers and analyzing data with xarray simply as a convenience library, this produces much too obfuscated code - a standard library import ( I think your earlier proposal of an |
{ "total_count": 3, "+1": 3, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 | |
242950070 | https://github.com/pydata/xarray/issues/988#issuecomment-242950070 | https://api.github.com/repos/pydata/xarray/issues/988 | MDEyOklzc3VlQ29tbWVudDI0Mjk1MDA3MA== | shoyer 1217238 | 2016-08-28T01:15:52Z | 2016-08-28T01:16:13Z | MEMBER | Let me give concrete examples of what this interface could look like. To implement units: ``` python from typing import List, Optional # optional Python 3.5 type annotations @xarray.register_ufunc_variables_attrs_handler def propagate_units(results: List[xarray.Variable], context: xarray.UFuncContext) -> Optional[List[dict]]: if context.func.name in ['add', 'sub']: units_set = set(getattr(arg, 'attrs', {}).get('units') for arg in context.args) if len(units_set) > 1: raise ValueError('not all input units the same: %r' % units_set) units, = units_set return [{'units': units}] else: return [] * len(results) # or equivalently, don't return anything at all ``` Or to (partially) handle
Or to implement
Every time xarray does an operation, we would call all of these registered
Similarly, we would have The 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. So alternatively to using the registration system, we could support/encourage using a context manager, e.g.,
It'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., ``` python import contextlib @contextlib.contextmanager def my_attrs_context(): with xarray.ufunc_variables_attrs_handlers( [always_keep_attrs, add_cell_methods, ...]): yield with my_attrs_context(): result = ds.mean() - 0.5 * (ds.max() - ds.min()) ``` So maybe a subclass based implementation (with a custom attribute like |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Hooks for custom attribute handling in xarray operations 173612265 |
Advanced export
JSON shape: default, array, newline-delimited, object
CREATE TABLE [issue_comments] ( [html_url] TEXT, [issue_url] TEXT, [id] INTEGER PRIMARY KEY, [node_id] TEXT, [user] INTEGER REFERENCES [users]([id]), [created_at] TEXT, [updated_at] TEXT, [author_association] TEXT, [body] TEXT, [reactions] TEXT, [performed_via_github_app] TEXT, [issue] INTEGER REFERENCES [issues]([id]) ); CREATE INDEX [idx_issue_comments_issue] ON [issue_comments] ([issue]); CREATE INDEX [idx_issue_comments_user] ON [issue_comments] ([user]);
user 9