issue_comments
27 rows where issue = 594594646 sorted by updated_at descending
This data as json, CSV (advanced)
Suggested facets: reactions, created_at (date), updated_at (date)
issue 1
- Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods · 27 ✖
id | html_url | issue_url | node_id | user | created_at | updated_at ▲ | author_association | body | reactions | performed_via_github_app | issue |
---|---|---|---|---|---|---|---|---|---|---|---|
651340318 | https://github.com/pydata/xarray/pull/3936#issuecomment-651340318 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDY1MTM0MDMxOA== | johnomotani 3958036 | 2020-06-29T20:22:49Z | 2020-06-29T20:22:49Z | CONTRIBUTOR | Thanks @dcherian :smile: You're very welcome! |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
651317938 | https://github.com/pydata/xarray/pull/3936#issuecomment-651317938 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDY1MTMxNzkzOA== | dcherian 2448579 | 2020-06-29T19:36:28Z | 2020-06-29T19:36:28Z | MEMBER | Thanks @johnomotani . This is a significant contribution. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
650303121 | https://github.com/pydata/xarray/pull/3936#issuecomment-650303121 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDY1MDMwMzEyMQ== | johnomotani 3958036 | 2020-06-26T17:29:15Z | 2020-06-26T17:29:15Z | CONTRIBUTOR | Thanks @keewis! I think we've addressed all the review comments now. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
650300879 | https://github.com/pydata/xarray/pull/3936#issuecomment-650300879 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDY1MDMwMDg3OQ== | johnomotani 3958036 | 2020-06-26T17:24:55Z | 2020-06-26T17:24:55Z | CONTRIBUTOR |
Some missing values should be OK. In your example though for example
I guess we could add some special handling for this (not sure what though, because we can't set a variable with type |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
650297994 | https://github.com/pydata/xarray/pull/3936#issuecomment-650297994 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDY1MDI5Nzk5NA== | keewis 14808389 | 2020-06-26T17:19:34Z | 2020-06-26T17:19:34Z | MEMBER | well, we'd need to somehow be able to use I think no further changes to the |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
650272329 | https://github.com/pydata/xarray/pull/3936#issuecomment-650272329 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDY1MDI3MjMyOQ== | johnomotani 3958036 | 2020-06-26T16:29:35Z | 2020-06-26T16:29:35Z | CONTRIBUTOR |
I think this is a bug. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
650221609 | https://github.com/pydata/xarray/pull/3936#issuecomment-650221609 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDY1MDIyMTYwOQ== | keewis 14808389 | 2020-06-26T14:51:51Z | 2020-06-26T14:51:51Z | MEMBER | I noticed a few issues while debugging:
- calling on a array with missing values raises |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
649204113 | https://github.com/pydata/xarray/pull/3936#issuecomment-649204113 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDY0OTIwNDExMw== | shoyer 1217238 | 2020-06-25T04:10:50Z | 2020-06-25T04:10:50Z | MEMBER | I'm going to wait a little while before merging in case anyone else has comment, but otherwise will merge in a day or two (definitely before the next release) |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
649059087 | https://github.com/pydata/xarray/pull/3936#issuecomment-649059087 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDY0OTA1OTA4Nw== | johnomotani 3958036 | 2020-06-24T20:39:31Z | 2020-06-24T20:39:31Z | CONTRIBUTOR | Merge conflicts fixed, this PR should be ready to review/merge. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
617381475 | https://github.com/pydata/xarray/pull/3936#issuecomment-617381475 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYxNzM4MTQ3NQ== | keewis 14808389 | 2020-04-21T19:57:27Z | 2020-04-21T20:48:41Z | MEMBER | you could mark the |
{ "total_count": 1, "+1": 1, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
617375991 | https://github.com/pydata/xarray/pull/3936#issuecomment-617375991 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYxNzM3NTk5MQ== | johnomotani 3958036 | 2020-04-21T19:46:09Z | 2020-04-21T19:46:09Z | CONTRIBUTOR |
|
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
617002459 | https://github.com/pydata/xarray/pull/3936#issuecomment-617002459 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYxNzAwMjQ1OQ== | shoyer 1217238 | 2020-04-21T07:23:11Z | 2020-04-21T07:23:27Z | MEMBER | I notice that some of your new deprecation warnings get issued when running your tests. Please silence these methods, e.g., using |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
616998622 | https://github.com/pydata/xarray/pull/3936#issuecomment-616998622 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYxNjk5ODYyMg== | shoyer 1217238 | 2020-04-21T07:14:10Z | 2020-04-21T07:14:10Z | MEMBER |
I guess this would happen if somebody calls If it's the former, then we should just fix xarray not to do this. If it's only in external code, I would still consider breaking this behavior. The override mechanism that NumPy uses in argmin/argmax is really old/fragile, and it's not something we ever intended to support. There's no way we could write an The better way to do overrides in NumPy is probably with |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
616100298 | https://github.com/pydata/xarray/pull/3936#issuecomment-616100298 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYxNjEwMDI5OA== | johnomotani 3958036 | 2020-04-19T10:42:06Z | 2020-04-19T10:42:06Z | CONTRIBUTOR | @pydata/xarray - I think this PR is ready to be merged, are there any changes I should make? Thanks! |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
612143069 | https://github.com/pydata/xarray/pull/3936#issuecomment-612143069 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYxMjE0MzA2OQ== | johnomotani 3958036 | 2020-04-10T17:51:13Z | 2020-04-10T17:51:13Z | CONTRIBUTOR | I eventually found that the cause of the errors I was getting was that the @shoyer I've kept the use of injected functions, because removing the injected I think this PR is ready for review now :smile: |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
610516784 | https://github.com/pydata/xarray/pull/3936#issuecomment-610516784 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYxMDUxNjc4NA== | shoyer 1217238 | 2020-04-07T17:23:24Z | 2020-04-07T17:23:24Z | MEMBER | Also, feel free to rewrite to avoid using inject_all_ops_and_reduce_methods() for argmin/argmax at all. Method injection is pretty hacky, and generally not worthwhile, e.g., see this note about removing it. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
610288219 | https://github.com/pydata/xarray/pull/3936#issuecomment-610288219 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYxMDI4ODIxOQ== | johnomotani 3958036 | 2020-04-07T09:45:24Z | 2020-04-07T09:45:24Z | CONTRIBUTOR | These test failures seem to have uncovered a larger issue: overriding a method injected by |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
610081717 | https://github.com/pydata/xarray/pull/3936#issuecomment-610081717 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYxMDA4MTcxNw== | johnomotani 3958036 | 2020-04-06T23:06:11Z | 2020-04-06T23:06:11Z | CONTRIBUTOR | The rename to |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
610079091 | https://github.com/pydata/xarray/pull/3936#issuecomment-610079091 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYxMDA3OTA5MQ== | johnomotani 3958036 | 2020-04-06T22:57:50Z | 2020-04-06T22:57:50Z | CONTRIBUTOR | I've updated so the new functionality is provided by |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
609607223 | https://github.com/pydata/xarray/pull/3936#issuecomment-609607223 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYwOTYwNzIyMw== | kmuehlbauer 5821660 | 2020-04-06T07:13:00Z | 2020-04-06T07:13:00Z | MEMBER | @johnomotani FYI: For #3871 (merged) there is #3922 (yet unmerged) to fix dask-handling. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
609605888 | https://github.com/pydata/xarray/pull/3936#issuecomment-609605888 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYwOTYwNTg4OA== | kmuehlbauer 5821660 | 2020-04-06T07:09:45Z | 2020-04-06T07:09:45Z | MEMBER |
@TomNicholas Probably I was bit confused by the current docstring. I think I understand now and there should be no problem at all. |
{ "total_count": 1, "+1": 1, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
609598095 | https://github.com/pydata/xarray/pull/3936#issuecomment-609598095 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYwOTU5ODA5NQ== | TomNicholas 35968931 | 2020-04-06T06:49:18Z | 2020-04-06T06:49:18Z | MEMBER |
That's a good question @kmuehlbauer, and the distinction probably needs to be clearer in the docs in general.
By this do you mean find the minimum as if the array were first (partially or totally) flattened along the given dims somehow? I'm not sure we provide that kind of behaviour anywhere in the current API. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
609591692 | https://github.com/pydata/xarray/pull/3936#issuecomment-609591692 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYwOTU5MTY5Mg== | kmuehlbauer 5821660 | 2020-04-06T06:31:12Z | 2020-04-06T06:31:22Z | MEMBER | +1 for making According to the current docstring it should already work that way for Reduce this DataArray’s data by applying argmin along some dimension(s). Returns: New DataArray/Dataset object with But this behaviour is broken currently (works only for one given dim). My main concern for changing the API as suggested above is, how should we discern (at least for
|
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
609488771 | https://github.com/pydata/xarray/pull/3936#issuecomment-609488771 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYwOTQ4ODc3MQ== | TomNicholas 35968931 | 2020-04-05T21:39:19Z | 2020-04-05T21:39:19Z | MEMBER |
+1 for overloading I also really like how neat this resultant property is
Although it's breaking and would require a deprecation cycle, I think this is what we should aim for.
Yes let's take the time to make that clearer for users - this will be a commonly-used function. |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
609477080 | https://github.com/pydata/xarray/pull/3936#issuecomment-609477080 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYwOTQ3NzA4MA== | johnomotani 3958036 | 2020-04-05T20:26:12Z | 2020-04-05T20:50:03Z | CONTRIBUTOR | @shoyer I think your last option sounds good. Questions:
* What should |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
609478091 | https://github.com/pydata/xarray/pull/3936#issuecomment-609478091 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYwOTQ3ODA5MQ== | johnomotani 3958036 | 2020-04-05T20:33:04Z | 2020-04-05T20:33:04Z | CONTRIBUTOR | Maybe worth noting, at the moment if you try to call
During handling of the above exception, another exception occurred: Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/<...>/xarray/xarray/core/common.py", line 46, in wrapped_func return self.reduce(func, dim, axis, skipna=skipna, kwargs) File "/<...>/xarray/xarray/core/dataarray.py", line 2288, in reduce var = self.variable.reduce(func, dim, axis, keep_attrs, keepdims, kwargs) File "/<...>/xarray/xarray/core/variable.py", line 1579, in reduce data = func(input_data, axis=axis, kwargs) File "/<...>/xarray/xarray/core/duck_array_ops.py", line 304, in f return func(values, axis=axis, kwargs) File "/<...>/xarray/xarray/core/duck_array_ops.py", line 47, in f return wrapped(args, kwargs) File "<array_function internals>", line 6, in argmin File "/<...>/anaconda3/lib/python3.7/site-packages/numpy/core/fromnumeric.py", line 1267, in argmin return _wrapfunc(a, 'argmin', axis=axis, out=out) File "/<...>/anaconda3/lib/python3.7/site-packages/numpy/core/fromnumeric.py", line 70, in _wrapfunc return _wrapit(obj, method, args, kwds) File "/<...>/anaconda3/lib/python3.7/site-packages/numpy/core/fromnumeric.py", line 47, in _wrapit result = getattr(asarray(obj), method)(*args, kwds) TypeError: 'tuple' object cannot be interpreted as an integer ``` |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 | |
609468040 | https://github.com/pydata/xarray/pull/3936#issuecomment-609468040 | https://api.github.com/repos/pydata/xarray/issues/3936 | MDEyOklzc3VlQ29tbWVudDYwOTQ2ODA0MA== | shoyer 1217238 | 2020-04-05T19:19:41Z | 2020-04-05T19:19:41Z | MEMBER | This looks really comprehensive, thank you! Before doing a really careful review here, I'd like to try to work out the full API design we want. I'll write out some of my thoughts here, but your thoughts would also be very welcome! Here's my summary of the current situation:
1. We have two pairs of methods, This PR implements the multidimensional equivalent of My first concern is about the name: it isn't obvious to me whether Another option would be to overload I think I like this last option best but I would be curious what others think! @pydata/xarray any thoughts on this? |
{ "total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0 } |
Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods 594594646 |
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 6