issue_comments

Table actions

8 rows where issue = 532940062 and user = 12862013 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 1

  • mark-boer · 8 ✖

issue 1

  • Add DataArray.pad, Dataset.pad, Variable.pad · 8 ✖

author_association 1

id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
602033234 https://github.com/pydata/xarray/pull/3596#issuecomment-602033234 https://api.github.com/repos/pydata/xarray/issues/3596 MDEyOklzc3VlQ29tbWVudDYwMjAzMzIzNA== mark-boer 12862013 2020-03-21T11:50:23Z 2020-03-21T11:50:44Z CONTRIBUTOR

Wow, didn't look for a week. Very happy 🎉! Thank you @dcherian and @max-sixty for all your input.

Must be one of the largest first contributions...

I was inspired by the hacktoberfest last year. Took a little bit over a month though 😛

 
{
    "total_count": 1,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 1,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add DataArray.pad, Dataset.pad, Variable.pad 532940062
596645749 https://github.com/pydata/xarray/pull/3596#issuecomment-596645749 https://api.github.com/repos/pydata/xarray/issues/3596 MDEyOklzc3VlQ29tbWVudDU5NjY0NTc0OQ== mark-boer 12862013 2020-03-09T16:48:11Z 2020-03-09T16:48:11Z CONTRIBUTOR

:) I think we need to extrapolate indexes by default. It seems like the most sensible option.

Sorry, obviously that is a solution ;-). But I do have some concerns:

In some instances extrapolating all coords, can lead to some unwanted behaviour. Would you suggest we only interpolate the indexes?

How would we handle unsorted indexes?

How would we extrapolate all the different kind of indexes, like the MultiIndex or CategoricalIndex?

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add DataArray.pad, Dataset.pad, Variable.pad 532940062
596240476 https://github.com/pydata/xarray/pull/3596#issuecomment-596240476 https://api.github.com/repos/pydata/xarray/issues/3596 MDEyOklzc3VlQ29tbWVudDU5NjI0MDQ3Ng== mark-boer 12862013 2020-03-08T19:09:17Z 2020-03-08T19:09:17Z CONTRIBUTOR

Edit: But it's more awkward for indexes than non-index coords. The index becomes less useful with non-unique values, and generally indexes don't have nulls. I'm not sure what the other options would be: to some extent it's the intersection of pad with xarray's data model.

Hmm, I don't really see a solution. What do you suggest? 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add DataArray.pad, Dataset.pad, Variable.pad 532940062
595498610 https://github.com/pydata/xarray/pull/3596#issuecomment-595498610 https://api.github.com/repos/pydata/xarray/issues/3596 MDEyOklzc3VlQ29tbWVudDU5NTQ5ODYxMA== mark-boer 12862013 2020-03-05T23:34:18Z 2020-03-05T23:34:18Z CONTRIBUTOR

The big outstanding issue is what to do about dimension coordinates or indexes. Currently this PR treats all variables in coords different from those in data_vars. I think this is confusing.

I do agree it can be confusing, but it is not unique in xarray. Dataset.shift only shifts data_vars, bfill and ffill only fill data_vars, etc.

Personally I think that extrapolating data coordinates without specifically settings a keyword or flag could also be confusing. I occasionally have data in my coords that make no sense to extrapolate. I do agree that adding an extrapolate_coord option or keyword of some sorts would be cool in the future.

Both defaults could be really useful, I'm still in a bit of doubt. Are there any other people that might want to weigh in?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add DataArray.pad, Dataset.pad, Variable.pad 532940062
585612496 https://github.com/pydata/xarray/pull/3596#issuecomment-585612496 https://api.github.com/repos/pydata/xarray/issues/3596 MDEyOklzc3VlQ29tbWVudDU4NTYxMjQ5Ng== mark-boer 12862013 2020-02-13T08:37:16Z 2020-02-13T08:37:16Z CONTRIBUTOR

Hi @dcherian

Once again an excellent code review :-)

I adressed most points in your review, except for the function signature of DataArray.pad. I think I prefer the default values of None. Now numpy/Dask will throw an error when you set invalid combinations e.g. constant_values=0, and set the mode="mean". The np.nan is unfortunate, but this is the difference between np.nan and xarray.dtypes.NaN. I could set the default to dtypes.NaN, but then you lose the automatic error when setting an explicit value with mode!="constant".

I can also see that something like coords_mode being useful, but I think it would be wise to wait and see how pad gets used and what issues we run into along the way.

I also ran into a small issue with pint (https://github.com/hgrecco/pint/issues/1026), but I'll get to that once this PR is merged.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add DataArray.pad, Dataset.pad, Variable.pad 532940062
578549057 https://github.com/pydata/xarray/pull/3596#issuecomment-578549057 https://api.github.com/repos/pydata/xarray/issues/3596 MDEyOklzc3VlQ29tbWVudDU3ODU0OTA1Nw== mark-boer 12862013 2020-01-26T22:16:42Z 2020-01-26T22:16:42Z CONTRIBUTOR

Hi @dcherian and @fujiisoup, appart from the issues I raised in my last comment, I think this PR is close to done.

Thx in advance

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add DataArray.pad, Dataset.pad, Variable.pad 532940062
570057684 https://github.com/pydata/xarray/pull/3596#issuecomment-570057684 https://api.github.com/repos/pydata/xarray/issues/3596 MDEyOklzc3VlQ29tbWVudDU3MDA1NzY4NA== mark-boer 12862013 2020-01-01T14:29:15Z 2020-01-01T14:29:15Z CONTRIBUTOR

Hi everyone, happy new year :tada:

I feel like this PR is slowly getting into a descent shape. But I could use your input on a couple of subjects:

How should we handle the default padding? I currently pad with dtypes.NaN this is similar behaviour as used by the replaced variable.pad_with_fill_valueand is consistent with theshift` method.

We could chose to pad with 0's as numpy does it, or we could force the user to choose a constantt_value, but I don't think I would prefer this.

How should the coordinates of a DataArray/set be padded? I chose default padding except for modes "edge", "reflect", "symmetric", "wrap". @dcherian noted that this could make sense in some situations, but that I would require some further discussion.

Personally I think it more often than not it makes sense, but padding with NaN's should also work fine.

dask_array_compat Also I created a workaround for the changing of dtype by Dask.pad mode=mean and added an additional check to validate the output shape of dask.pad. Let me know what you think about this, I'm not 100% convinced...

I used a couple of # type: ignore as any work around the mypy errors lead to uglier code. But if any of you have some suggestions, I'm happy to hear them

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add DataArray.pad, Dataset.pad, Variable.pad 532940062
564904765 https://github.com/pydata/xarray/pull/3596#issuecomment-564904765 https://api.github.com/repos/pydata/xarray/issues/3596 MDEyOklzc3VlQ29tbWVudDU2NDkwNDc2NQ== mark-boer 12862013 2019-12-12T08:35:06Z 2019-12-12T08:35:06Z CONTRIBUTOR

It seems like we have some value mismatches on dask==1.2.

The reason that mean and reflect are marked as xfail are because there too there are value mismatches. I'll see if I can find a workaround for this failing test.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add DataArray.pad, Dataset.pad, Variable.pad 532940062

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]);