home / github

Menu
  • GraphQL API
  • Search all tables

issue_comments

Table actions
  • GraphQL API for issue_comments

13 rows where author_association = "CONTRIBUTOR", issue = 594594646 and user = 3958036 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

Suggested facets: created_at (date), updated_at (date)

user 1

  • johnomotani · 13 ✖

issue 1

  • Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods · 13 ✖

author_association 1

  • CONTRIBUTOR · 13 ✖
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
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

calling on a array with missing values raises ValueError: All-NaN slice encountered (try ds = xr.tutorial.open_dataset("rasm"); ds.argmin(dim="y"))

Some missing values should be OK. In your example though for example In [27]: ds.Tair.isel(time=0, x=0).values Out[27]: array([nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan]) So there are no values in the array slice that argmin should be applied over, which is an error.

I guess we could add some special handling for this (not sure what though, because we can't set a variable with type int to nan or None), but I think that is a separate issue that would need a new PR.

{
    "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

ds.argmin(dim=...) returns a single index (the same result as ds.argmin()) but ds.Tair.argmin(dim=...) returns something different from ds.Tair.argmin(). Is that intentional?

I think this is a bug. dim=... to Dataset.argmin should be an error because dim=... returns a dict of results, and it's not clear how to do that consistently for a Dataset that might have several members with different combinations of dimensions.

{
    "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
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

I guess this would happen if somebody calls np.argmin(xarray_object)? Does this happen from inside xarray somewhere, or in external code?

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.

test_units.py has a couple of tests that apply numpy functions to xr.Variables, xr.DataArrays and xr.Datasets. Those tests break for argmin and argmax without the out argument. Should I just remove the argmin and argmax function calls? The tests would still call them as methods.

{
    "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 argmin and argmax methods did not have an out argument. In order for the methods to be wrapped by numpy (and I guess dask is the same), the call signature of np.argmin() has to be supported, which means axis and out arguments are needed.

@shoyer I've kept the use of injected functions, because removing the injected argmin and argmax completely meant re-implementing handling of skipna and only within _unravel_argminmax, which seemed less nice to me. If/when method injection is refactored, it would be nice to include a mechanism to override the core operation with an explicitly implemented, extended version like argmin/argmax.

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
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 ops.inject_all_ops_and_reduce_methods(DataArray, priority=60) is tricky. I think there may be a way around it by mangling the name of the injected operation if that method is already defined on the class; I tried this and it nearly works, but I think I need to implement an override for argmin/argmax on Variable, because DataArray.__array_wrap__ uses the Variable version of the method... Work in progress!

{
    "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 _argmin_base is also causing test_aggregation (which is a test of units with pint) to fail, because the numpy function and the Variable method do not have the same name. Any suggestions how to fix this? It seems tricky because I can't pass either argmin (no Variable method) or _argmin_base (no numpy method) to the test.

{
    "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 argmin() and argmax(), when they are passed a multiple dimensions. * I've added Dataset.argmin() and Dataset.argmax(), which only work for a single dimension, but give informative exceptions for unsupported cases. If anyone has some behaviour that they would like for Dataset.argmin() with multiple dimensions, I'm happy to implement it, but returning something like a dict of Datasets didn't seem nice, and as far as I understand a Dataset cannot contain something like a dict of DataArrays. * I renamed argmin and argmax in ops.py and duck_array_ops.py to _argmin_base and _argmax_base so that I could still use them inside the argmin and argmax methods. Any alternatives, or better suggestions for naming them? * Variable no longer has argmin or argmax methods. Is that a problem? I just updated test_variable.py and test_dask.py to use _argmin_base and _argmax_base.

{
    "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 da.argmin() with no arguments do? * Currently returns the flattened index of the global minimum. * I think returning a dict of indices would be much more useful, but it does change existing behaviour (more useful because you can then do da.isel(da.argmin())). * Could anyway do da.argmin(da.dims) to get the dict result, but that's a bit ugly * Could have something like da.argmin(...) - maybe as a temporary workaround while we deprecate the current behaviour of da.argmin()? * If we overload argmin, what's the cleanest way to get the existing argmin() method within the new one? Would something like from .duck_array_ops import argmin as argmin_1d work?

{
    "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 argmin(dim=("x", "y")) with multiple dimensions, there's a not-very-helpful exception ```

array.argmin(dim=("x", "y")) Traceback (most recent call last): File "/home/john/anaconda3/lib/python3.7/site-packages/numpy/core/fromnumeric.py", line 61, in _wrapfunc return bound(args, *kwds) TypeError: 'tuple' object cannot be interpreted as an integer

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

Advanced export

JSON shape: default, array, newline-delimited, object

CSV options:

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]);
Powered by Datasette · Queries took 18.003ms · About: xarray-datasette