home / github

Menu
  • GraphQL API
  • Search all tables

issue_comments

Table actions
  • GraphQL API for issue_comments

11 rows where issue = 879033384 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

user 6

  • max-sixty 3
  • seth-p 3
  • keewis 2
  • dcherian 1
  • Illviljan 1
  • raybellwaves 1

author_association 2

  • MEMBER 7
  • CONTRIBUTOR 4

issue 1

  • DataArray.clip() no longer supports the out argument · 11 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
856340566 https://github.com/pydata/xarray/issues/5278#issuecomment-856340566 https://api.github.com/repos/pydata/xarray/issues/5278 MDEyOklzc3VlQ29tbWVudDg1NjM0MDU2Ng== seth-p 7441788 2021-06-08T00:02:48Z 2021-06-08T00:04:28Z CONTRIBUTOR

almost no attention is paid to minimizing memory consumption (whether through in-place operations, or more generally minimizing temporary memory usage).

I think we'd be open to fixing this when it doesn't compromise readability. Can you open a new issue with some particularly bad examples?

I wouldn't necessarily say that it's particularly bad, but see the discussion following https://github.com/pydata/xarray/pull/2922#issuecomment-601496897.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  DataArray.clip() no longer supports the out argument 879033384
835886774 https://github.com/pydata/xarray/issues/5278#issuecomment-835886774 https://api.github.com/repos/pydata/xarray/issues/5278 MDEyOklzc3VlQ29tbWVudDgzNTg4Njc3NA== dcherian 2448579 2021-05-09T20:53:16Z 2021-05-09T20:53:16Z MEMBER

almost no attention is paid to minimizing memory consumption (whether through in-place operations, or more generally minimizing temporary memory usage).

I think we'd be open to fixing this when it doesn't compromise readability. Can you open a new issue with some particularly bad examples?

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  DataArray.clip() no longer supports the out argument 879033384
835886145 https://github.com/pydata/xarray/issues/5278#issuecomment-835886145 https://api.github.com/repos/pydata/xarray/issues/5278 MDEyOklzc3VlQ29tbWVudDgzNTg4NjE0NQ== Illviljan 14371165 2021-05-09T20:50:40Z 2021-05-09T20:50:40Z MEMBER

xarray allows using dask arrays for data that don’t fit into memory and dask doesn't support out, https://docs.dask.org/en/latest/array-api.html#dask.array.clip.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  DataArray.clip() no longer supports the out argument 879033384
835597488 https://github.com/pydata/xarray/issues/5278#issuecomment-835597488 https://api.github.com/repos/pydata/xarray/issues/5278 MDEyOklzc3VlQ29tbWVudDgzNTU5NzQ4OA== seth-p 7441788 2021-05-09T00:40:08Z 2021-05-09T00:40:08Z CONTRIBUTOR

I'm not familiar at all with the various numpy interfaces, so I can't offer any input implementation-wise. But as a user, being able to do operations in place (via out or otherwise) is extremely useful when dealing with large arrays under memory constraints. In fact my one "philosophical" beef with xarray is that it seems almost no attention is paid to minimizing memory consumption (whether through in-place operations, or more generally minimizing temporary memory usage).

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  DataArray.clip() no longer supports the out argument 879033384
835562066 https://github.com/pydata/xarray/issues/5278#issuecomment-835562066 https://api.github.com/repos/pydata/xarray/issues/5278 MDEyOklzc3VlQ29tbWVudDgzNTU2MjA2Ng== keewis 14808389 2021-05-08T23:04:30Z 2021-05-08T23:04:30Z MEMBER

__array_wrap__ doesn't allow specifying any kwargs (as far as I can tell), it has two parameters: obj and context. I'd have to read up on this, but obj seems to be the data (from asarray or __array_prepare__?) and context seems to be None most of the time.

In any case, I think we should try to replace __array_wrap__ with __array_function__ (or __array_module__ / __array_namespace__?) soon.

Also, I agree on being consistent regarding the out parameter.

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  DataArray.clip() no longer supports the out argument 879033384
835558405 https://github.com/pydata/xarray/issues/5278#issuecomment-835558405 https://api.github.com/repos/pydata/xarray/issues/5278 MDEyOklzc3VlQ29tbWVudDgzNTU1ODQwNQ== max-sixty 5635139 2021-05-08T22:55:17Z 2021-05-08T22:55:17Z MEMBER

Thanks @keewis , that makes sense.

And ensuring I return to @seth-p 's original issue — I would be up for adding out if we're violating the __array_wrap__ standard by not having all its kwargs.

Otherwise I would vote weakly against out throughout xarray's API, and strongly against it out on an arbitrary subset of methods.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  DataArray.clip() no longer supports the out argument 879033384
835518391 https://github.com/pydata/xarray/issues/5278#issuecomment-835518391 https://api.github.com/repos/pydata/xarray/issues/5278 MDEyOklzc3VlQ29tbWVudDgzNTUxODM5MQ== max-sixty 5635139 2021-05-08T21:18:14Z 2021-05-08T22:37:28Z MEMBER

Thanks for the example @raybellwaves

Supporting np.{func} would be good — I have only partially followed the interop with numpy — e.g. __array_ufunc__ (edit: __array_function__) — is anyone familiar with whether we're violating the standards by not having an out arg with clip?

FYI, we don't generally have out args, but we do have them in nanops.py:

xarray/core/nanops.py: 78: def nanmin(a, axis=None, out=None): 86: def nanmax(a, axis=None, out=None): 112: def nansum(a, axis=None, dtype=None, out=None, min_count=None): 137: def nanmean(a, axis=None, dtype=None, out=None): 151: def nanmedian(a, axis=None, out=None): 169: def nanvar(a, axis=None, dtype=None, out=None, ddof=0): 178: def nanstd(a, axis=None, dtype=None, out=None, ddof=0): 184: def nanprod(a, axis=None, dtype=None, out=None, min_count=None): 193: def nancumsum(a, axis=None, dtype=None, out=None): 199: def nancumprod(a, axis=None, dtype=None, out=None):

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  DataArray.clip() no longer supports the out argument 879033384
835529368 https://github.com/pydata/xarray/issues/5278#issuecomment-835529368 https://api.github.com/repos/pydata/xarray/issues/5278 MDEyOklzc3VlQ29tbWVudDgzNTUyOTM2OA== keewis 14808389 2021-05-08T21:45:32Z 2021-05-08T21:46:35Z MEMBER

clip dispatches through __array_function__ (it's not a real "ufunc"), so we don't really support that (there's a issue for adding __array_function__, but no progress yet). In general I think we should only support functions which don't have a axis parameter and return NotImplemented for all others, which means clip would be a good candidate.

We currently do have some support for numpy.clip(arr, ...) through __array_wrap__ (so no support for Dataset), not sure if that even allows deviating from a standard?

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  DataArray.clip() no longer supports the out argument 879033384
835499783 https://github.com/pydata/xarray/issues/5278#issuecomment-835499783 https://api.github.com/repos/pydata/xarray/issues/5278 MDEyOklzc3VlQ29tbWVudDgzNTQ5OTc4Mw== raybellwaves 17162724 2021-05-08T20:26:51Z 2021-05-08T20:26:51Z CONTRIBUTOR

FWIW. This showed up in xskillscore as we were doing np.clip(xarray object, min, min). We updated the code to do xarray object.clip(min, max) which we probably should have been doing in the first place (https://github.com/xarray-contrib/xskillscore/pull/309#issue-634256273)

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  DataArray.clip() no longer supports the out argument 879033384
834629871 https://github.com/pydata/xarray/issues/5278#issuecomment-834629871 https://api.github.com/repos/pydata/xarray/issues/5278 MDEyOklzc3VlQ29tbWVudDgzNDYyOTg3MQ== seth-p 7441788 2021-05-07T17:11:01Z 2021-05-07T17:11:14Z CONTRIBUTOR

What is the case for having out kwargs?

It lets you reuse memory you already have. In particular for a simple operation like clip, you can do it in-place: da.clip(..., out=da.values). Very useful if you deal with lots of data and memory is a concern.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  DataArray.clip() no longer supports the out argument 879033384
834591933 https://github.com/pydata/xarray/issues/5278#issuecomment-834591933 https://api.github.com/repos/pydata/xarray/issues/5278 MDEyOklzc3VlQ29tbWVudDgzNDU5MTkzMw== max-sixty 5635139 2021-05-07T16:23:50Z 2021-05-07T16:23:50Z MEMBER

FYI I'm out on paternity leave for a few weeks.

Generally IIRC out hasn't been part of the public API, and only present because of our delegating to numpy for some functions.

What is the case for having out kwargs?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  DataArray.clip() no longer supports the out argument 879033384

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 238.206ms · About: xarray-datasette