home / github

Menu
  • GraphQL API
  • Search all tables

issue_comments

Table actions
  • GraphQL API for issue_comments

17 rows where author_association = "MEMBER" and issue = 356698348 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 5

  • shoyer 7
  • fujiisoup 6
  • fmaussion 2
  • rabernat 1
  • spencerkclark 1

issue 1

  • implement Gradient · 17 ✖

author_association 1

  • MEMBER · 17 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
423167921 https://github.com/pydata/xarray/pull/2398#issuecomment-423167921 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMzE2NzkyMQ== fujiisoup 6815844 2018-09-20T12:40:16Z 2018-09-20T12:40:16Z MEMBER

Thanks, @rabernat for the review.

Added the limitation of this method to docs.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
423017107 https://github.com/pydata/xarray/pull/2398#issuecomment-423017107 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMzAxNzEwNw== shoyer 1217238 2018-09-20T02:14:25Z 2018-09-20T02:14:25Z MEMBER

@rabernat let me know if this makes sense to you / if you agree.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
422980788 https://github.com/pydata/xarray/pull/2398#issuecomment-422980788 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMjk4MDc4OA== shoyer 1217238 2018-09-19T22:33:58Z 2018-09-19T22:33:58Z MEMBER

I'm going to merge this after the Appveyor tests pass, unless there are any further objections.

{
    "total_count": 2,
    "+1": 2,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
422978507 https://github.com/pydata/xarray/pull/2398#issuecomment-422978507 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMjk3ODUwNw== spencerkclark 6628425 2018-09-19T22:23:23Z 2018-09-19T22:23:23Z MEMBER

I added this function for something like this extension, though I do not yet fully follow your cftime update. It would be super nice if you could take care of this after merge.

I totally understand; cftime objects probably seem fairly esoteric to non climate scientists. I'll be happy to take care of this after this gets merged. Indeed having the relevant logic already contained in to_numeric is very convenient.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
422973404 https://github.com/pydata/xarray/pull/2398#issuecomment-422973404 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMjk3MzQwNA== fujiisoup 6815844 2018-09-19T22:01:12Z 2018-09-19T22:01:12Z MEMBER

Thanks all. Updated.

Thanks, @spencerkclark

Eventually it would be nice if this worked on DataArrays with cftime.datetime coordinates; I think it would be relatively straightforward to modify to_numeric to enable it (we could probably enable it for interp at the same time), but I can take care of that later if you'd like.

Thanks. I added this function for something like this extension, though I do not yet fully follow your cftime update. It would be super nice if you could take care of this after merge.

@shoyer ,

we might want to include an option for periodic boundary conditions

Agreed. This option is nice not only differentiate but also interp and rolling. I think we can add a common logic to take care of them.

@fmaussion

See also #1288 : integrate is the next on my list ;) - I can try to give it a go if @fujiisoup doesn't want to do it himself

Thanks. Actually, I am now moving to another nation and would not have enough time in a few weeks. I will appreciate if you could take care of this.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
422949281 https://github.com/pydata/xarray/pull/2398#issuecomment-422949281 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMjk0OTI4MQ== shoyer 1217238 2018-09-19T20:36:15Z 2018-09-19T20:36:15Z MEMBER

We should definitely mention specialized tools like xgcm as alternatives in the docstring for these xarray methods.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
422949121 https://github.com/pydata/xarray/pull/2398#issuecomment-422949121 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMjk0OTEyMQ== shoyer 1217238 2018-09-19T20:35:43Z 2018-09-19T20:35:43Z MEMBER

@rabernat my inclination would be to draw the line at regular grids (with or without periodic boundary conditions), which are pretty commonly encountered in data analysis for any continuous physical systems. We would leave non-regular cases like staggered-grids to add-ons like xgcm -- these grids tend to be more application/PDE specific.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
422947647 https://github.com/pydata/xarray/pull/2398#issuecomment-422947647 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMjk0NzY0Nw== rabernat 1197350 2018-09-19T20:30:58Z 2018-09-19T20:30:58Z MEMBER

I agree that the features implemented here are broadly useful and belong in xarray. The fundamental question is: how far does xarray itself want to go in supporting vector calculus in curvilinear coordinates (i.e. on the sphere).

There is a fair bit of overlap between this new functionality and some of the things that we are trying to do in xgcm: https://xgcm.readthedocs.io/en/latest/grids.html. Xgcm supports periodic boundary conditions, as well as more complex topological connections between array edges. It allows users to reproduce precisely the sort of operations used to take gradients in finite-volume staggered-grid models.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
422940673 https://github.com/pydata/xarray/pull/2398#issuecomment-422940673 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMjk0MDY3Mw== shoyer 1217238 2018-09-19T20:07:35Z 2018-09-19T20:07:35Z MEMBER

@shoyer, have you seen da.pad?

Yes, pad solves the hard part of periodic boundary conditions -- we should just need to do a bit of book-keeping on the xarray side.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
422864322 https://github.com/pydata/xarray/pull/2398#issuecomment-422864322 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMjg2NDMyMg== fmaussion 10050469 2018-09-19T16:14:48Z 2018-09-19T16:14:48Z MEMBER

But we can save that for another PR :)

See also https://github.com/pydata/xarray/issues/1288 : integrate is the next on my list ;) - I can try to give it a go if @fujiisoup doesn't want to do it himself

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
422863296 https://github.com/pydata/xarray/pull/2398#issuecomment-422863296 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMjg2MzI5Ng== shoyer 1217238 2018-09-19T16:11:51Z 2018-09-19T16:11:51Z MEMBER

Speaking of derivatives on the globe, we might want to include an option for periodic boundary conditions (and possibly for other functions like interp as well). But we can save that for another PR :).

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
422860446 https://github.com/pydata/xarray/pull/2398#issuecomment-422860446 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMjg2MDQ0Ng== fmaussion 10050469 2018-09-19T16:03:55Z 2018-09-19T16:03:55Z MEMBER

This is so great! This is going to simplify my classes about derivatives on the globe even more: my only concern is that students will soon forget what a numerical derivative actually is, and that it's not a trivial to implement ;-)

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
422639302 https://github.com/pydata/xarray/pull/2398#issuecomment-422639302 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMjYzOTMwMg== fujiisoup 6815844 2018-09-19T03:35:51Z 2018-09-19T03:35:51Z MEMBER

I think it's ready :)

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
420474101 https://github.com/pydata/xarray/pull/2398#issuecomment-420474101 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMDQ3NDEwMQ== fujiisoup 6815844 2018-09-12T00:49:23Z 2018-09-12T00:49:23Z MEMBER

Thanks, @dopplershift .

Aren't you taking differences of values and dividing by differences between the matching coordinates?

Yes, correct. But if we have closer data points, then the estimate of the gradient becomes more precise. Sorting the array according to the coordinate provides the closest points, resulting in the most precise estimate of the gradient.

But I also think users can do it manually before taking the gradient.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
420458057 https://github.com/pydata/xarray/pull/2398#issuecomment-420458057 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQyMDQ1ODA1Nw== fujiisoup 6815844 2018-09-11T23:18:17Z 2018-09-11T23:18:17Z MEMBER

any thoughts for this?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
418541019 https://github.com/pydata/xarray/pull/2398#issuecomment-418541019 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQxODU0MTAxOQ== fujiisoup 6815844 2018-09-04T22:41:41Z 2018-09-04T22:41:41Z MEMBER

I wonder if we should consider calling this xarray.differentiate instead of xarray.gradient. I think the NumPy function is poorly named for differentiating along a single axis at once.

Agreed. Differentiate is nicer.

Some other api questions arised during the implementation + Do we support differentiate for Dataset? In that case, what should we do for the variables that are independent from the target coordinate? I thought 'keep them as is' is intuitive (and I implemented so), but mathematically, they should be zero.

  • Do we need to sort the array before computing differentiate? np.gradient implicitly assumes the array is sorted (but do nothing about this).
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348
418530033 https://github.com/pydata/xarray/pull/2398#issuecomment-418530033 https://api.github.com/repos/pydata/xarray/issues/2398 MDEyOklzc3VlQ29tbWVudDQxODUzMDAzMw== shoyer 1217238 2018-09-04T21:51:44Z 2018-09-04T21:51:44Z MEMBER

I think this will be very welcome functionality!

I wonder if we should consider calling this xarray.differentiate instead of xarray.gradient. I think the NumPy function is poorly named for differentiating along a single axis at once.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Gradient 356698348

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