issue_comments

Table actions

11 rows where issue = 381518663 sorted by updated_at descending

✖
✖

✎ View and edit SQL

This data as json, CSV (advanced)

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

author_association 3

issue 1

  • Add transpose_coords option to DataArray.transpose · 11 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
494487640 https://github.com/pydata/xarray/pull/2556#issuecomment-494487640 https://api.github.com/repos/pydata/xarray/issues/2556 MDEyOklzc3VlQ29tbWVudDQ5NDQ4NzY0MA== shoyer 1217238 2019-05-21T17:38:07Z 2019-05-21T17:38:07Z MEMBER

OK, in it goes!

Thanks for your patience @phausamann !

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add transpose_coords option to DataArray.transpose 381518663
494478111 https://github.com/pydata/xarray/pull/2556#issuecomment-494478111 https://api.github.com/repos/pydata/xarray/issues/2556 MDEyOklzc3VlQ29tbWVudDQ5NDQ3ODExMQ== phausamann 4687996 2019-05-21T17:10:59Z 2019-05-21T17:10:59Z CONTRIBUTOR

Looks like the build failures were only temporary, all the checks are passing now. I think this is good to go.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add transpose_coords option to DataArray.transpose 381518663
439348478 https://github.com/pydata/xarray/pull/2556#issuecomment-439348478 https://api.github.com/repos/pydata/xarray/issues/2556 MDEyOklzc3VlQ29tbWVudDQzOTM0ODQ3OA== pep8speaks 24736507 2018-11-16T10:20:19Z 2019-05-21T16:42:27Z NONE

Hello @phausamann! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2019-05-21 16:42:27 UTC
 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add transpose_coords option to DataArray.transpose 381518663
488199163 https://github.com/pydata/xarray/pull/2556#issuecomment-488199163 https://api.github.com/repos/pydata/xarray/issues/2556 MDEyOklzc3VlQ29tbWVudDQ4ODE5OTE2Mw== shoyer 1217238 2019-05-01T03:28:12Z 2019-05-01T03:28:12Z MEMBER

I just merged in master, let's see how that goes

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add transpose_coords option to DataArray.transpose 381518663
488116448 https://github.com/pydata/xarray/pull/2556#issuecomment-488116448 https://api.github.com/repos/pydata/xarray/issues/2556 MDEyOklzc3VlQ29tbWVudDQ4ODExNjQ0OA== StanczakDominik 11289391 2019-04-30T21:08:38Z 2019-04-30T21:08:38Z CONTRIBUTOR

Hi, just popping in - perhaps rebasing on top of the master branch would help move this along?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add transpose_coords option to DataArray.transpose 381518663
476281443 https://github.com/pydata/xarray/pull/2556#issuecomment-476281443 https://api.github.com/repos/pydata/xarray/issues/2556 MDEyOklzc3VlQ29tbWVudDQ3NjI4MTQ0Mw== phausamann 4687996 2019-03-25T16:41:32Z 2019-03-25T16:41:32Z CONTRIBUTOR

@shoyer is this ready to be merged?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add transpose_coords option to DataArray.transpose 381518663
470940357 https://github.com/pydata/xarray/pull/2556#issuecomment-470940357 https://api.github.com/repos/pydata/xarray/issues/2556 MDEyOklzc3VlQ29tbWVudDQ3MDk0MDM1Nw== phausamann 4687996 2019-03-08T14:08:46Z 2019-03-08T14:08:46Z CONTRIBUTOR

I've added a restore_coord_dims parameter to groupby, groupby_bins and resample and eliminated all warnings from the tests. The build failure seems to be inherited from the master branch.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add transpose_coords option to DataArray.transpose 381518663
441019330 https://github.com/pydata/xarray/pull/2556#issuecomment-441019330 https://api.github.com/repos/pydata/xarray/issues/2556 MDEyOklzc3VlQ29tbWVudDQ0MTAxOTMzMA== phausamann 4687996 2018-11-22T12:42:59Z 2018-11-22T12:43:34Z CONTRIBUTOR

I've changed all the transpose calls in the tests to transpose_coords=True. Other than that, there's some transpose calls in plot and groupby that I've updated, but I'm not sure whether they should be True, False or not updated at all (see above).

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add transpose_coords option to DataArray.transpose 381518663
440362603 https://github.com/pydata/xarray/pull/2556#issuecomment-440362603 https://api.github.com/repos/pydata/xarray/issues/2556 MDEyOklzc3VlQ29tbWVudDQ0MDM2MjYwMw== shoyer 1217238 2018-11-20T17:34:33Z 2018-11-20T17:34:33Z MEMBER

I'm not exactly sure how this works, there are a lot of tests where the warning is issued, especially calls to DataArray.T. Should I update all of these tests to transpose(..., transpose_coords=False)?

Yes, that would probably be the right call. Or maybe even switch to .transpose(transpose_coords=True) to future proof the behavior?

We try to avoid issuing warnings in xarray's own test suite so we notice issues (e.g., deprecations in upstream packages) when they arise. This also gives us some estimate of how much turmoil a change will cause for users -- if we end up with lots of internal warnings, then users will probably notice, too.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add transpose_coords option to DataArray.transpose 381518663
439918296 https://github.com/pydata/xarray/pull/2556#issuecomment-439918296 https://api.github.com/repos/pydata/xarray/issues/2556 MDEyOklzc3VlQ29tbWVudDQzOTkxODI5Ng== phausamann 4687996 2018-11-19T14:50:30Z 2018-11-19T14:50:30Z CONTRIBUTOR

I've updated the method to issue the warning and fixed the failing tests (which was actually a bug in my implementation).

It would be fine to use transpose_coords=False explicitly in tests, but we should silence all warnings.

I'm not exactly sure how this works, there are a lot of tests where the warning is issued, especially calls to DataArray.T. Should I update all of these tests to transpose(..., transpose_coords=False)?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add transpose_coords option to DataArray.transpose 381518663
439476589 https://github.com/pydata/xarray/pull/2556#issuecomment-439476589 https://api.github.com/repos/pydata/xarray/issues/2556 MDEyOklzc3VlQ29tbWVudDQzOTQ3NjU4OQ== shoyer 1217238 2018-11-16T18:02:52Z 2018-11-16T18:02:52Z MEMBER

As a default, I think we want transpose_coords=None.

If transpose_coords is None and there are coordinates to transpose, we can issue a FutureWarning about how the default behavior of transpose_coords will switch to True in the future.

I'm not sure whether fixing these tests to work with the new behavior should be part of the PR.

Yes, this would be ideal. It would be fine to use transpose_coords=False explicitly in tests, but we should silence all warnings.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Add transpose_coords option to DataArray.transpose 381518663

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