issue_comments

Table actions

8 rows where issue = 913830070 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

  • fix dask meta and output_dtypes error · 8 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
856543812 https://github.com/pydata/xarray/pull/5449#issuecomment-856543812 https://api.github.com/repos/pydata/xarray/issues/5449 MDEyOklzc3VlQ29tbWVudDg1NjU0MzgxMg== kmuehlbauer 5821660 2021-06-08T07:51:49Z 2021-06-08T07:51:49Z MEMBER

Thanks for the ping @dcherian.

There is this test: https://github.com/pydata/xarray/blob/da0489f4c7c823915dbb52b681b867baa16bc6e3/xarray/tests/test_computation.py#L1314-L1330 which provides dtype in meta. Maybe these different tests have to be aligned/combined? Unfortunately memory gets hazy over time, so I can't say much more about it without a deeper dive into.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   fix dask meta and output_dtypes error 913830070
856529731 https://github.com/pydata/xarray/pull/5449#issuecomment-856529731 https://api.github.com/repos/pydata/xarray/issues/5449 MDEyOklzc3VlQ29tbWVudDg1NjUyOTczMQ== mathause 10194086 2021-06-08T07:30:24Z 2021-06-08T07:30:24Z MEMBER

No worries. I agree that only allowing one is cleaner. And definitively +1 on adding tests using meta.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   fix dask meta and output_dtypes error 913830070
856294922 https://github.com/pydata/xarray/pull/5449#issuecomment-856294922 https://api.github.com/repos/pydata/xarray/issues/5449 MDEyOklzc3VlQ29tbWVudDg1NjI5NDkyMg== dcherian 2448579 2021-06-07T22:05:55Z 2021-06-07T22:05:55Z MEMBER

but without much knowledge of how downstream libraries were using these arguments.

IIRC xarray doesn't actually use it. xarray.apply_ufunc is a convenient wrapper for apply_gufunc so we just pass along what the user provides.

@kmuehlbauer added that test so maybe he can provide more context

Perhaps removing one of output_dtypes or meta from the test would be the best solution.

OK good point! There are a few tests for output_dtypes but we don't actually test for providing dtype in meta, so perhaps we should do that. 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   fix dask meta and output_dtypes error 913830070
856285749 https://github.com/pydata/xarray/pull/5449#issuecomment-856285749 https://api.github.com/repos/pydata/xarray/issues/5449 MDEyOklzc3VlQ29tbWVudDg1NjI4NTc0OQ== gjoseph92 3309802 2021-06-07T21:45:30Z 2021-06-07T21:45:30Z NONE

@mathause sorry for breaking things here. Note that passing output_dtypes didn't work as it was supposed to before, and also didn't cause a cast. We went back and forth on whether output_types should cause explicit casting, and whether it was sensible to provide both it and meta. Ultimately we decided they should be mutually exclusive, and should not cause casting, but without much knowledge of how downstream libraries were using these arguments. So maybe we should revisit that choice in dask?

Also I think maybe this test should be changed rather than skipped. Saying output_dtypes=[int] and then assert float == actual.dtype just seems weird to me. Perhaps removing one of output_dtypes or meta from the test would be the best solution.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   fix dask meta and output_dtypes error 913830070
856285638 https://github.com/pydata/xarray/pull/5449#issuecomment-856285638 https://api.github.com/repos/pydata/xarray/issues/5449 MDEyOklzc3VlQ29tbWVudDg1NjI4NTYzOA== jrbourbeau 11656932 2021-06-07T21:45:15Z 2021-06-07T21:45:15Z CONTRIBUTOR

Apologies for the breaking change. I'm happy to make sure we ping an Xarray dev next time we have a significant change to apply_gufunc to avoid surprises like this in the future. 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   fix dask meta and output_dtypes error 913830070
856272615 https://github.com/pydata/xarray/pull/5449#issuecomment-856272615 https://api.github.com/repos/pydata/xarray/issues/5449 MDEyOklzc3VlQ29tbWVudDg1NjI3MjYxNQ== github-actions[bot] 41898282 2021-06-07T21:25:56Z 2021-06-07T21:25:56Z CONTRIBUTOR

Unit Test Results

0 files  ±0  0 suites  ±0   0s :stopwatch: ±0s 0 tests ±0  0 :heavy_check_mark: ±0  0 :zzz: ±0  0 :x: ±0 

Results for commit 34dc5771. ± Comparison against base commit 34dc5771.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   fix dask meta and output_dtypes error 913830070
856263319 https://github.com/pydata/xarray/pull/5449#issuecomment-856263319 https://api.github.com/repos/pydata/xarray/issues/5449 MDEyOklzc3VlQ29tbWVudDg1NjI2MzMxOQ== max-sixty 5635139 2021-06-07T21:17:32Z 2021-06-07T21:17:32Z MEMBER

Thanks @mathause !

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   fix dask meta and output_dtypes error 913830070
856256166 https://github.com/pydata/xarray/pull/5449#issuecomment-856256166 https://api.github.com/repos/pydata/xarray/issues/5449 MDEyOklzc3VlQ29tbWVudDg1NjI1NjE2Ng== mathause 10194086 2021-06-07T21:04:38Z 2021-06-07T21:04:38Z MEMBER

thanks for the review - let's merge soon

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   fix dask meta and output_dtypes error 913830070

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