issue_comments

Table actions

14 rows where issue = 490316894 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

author_association 3

issue 1

  • Hypothesis tests for roundtrip to & from pandas · 14 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
547996494 https://github.com/pydata/xarray/pull/3285#issuecomment-547996494 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0Nzk5NjQ5NA== max-sixty 5635139 2019-10-30T16:33:37Z 2019-10-30T16:33:37Z MEMBER

Thanks @takluyver. And @Zac-HD for the feedback; v much agree with your approach

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
547932332 https://github.com/pydata/xarray/pull/3285#issuecomment-547932332 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0NzkzMjMzMg== dcherian 2448579 2019-10-30T14:28:46Z 2019-10-30T14:28:46Z MEMBER

Opened #3468 . Thanks @takluyver 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
547837799 https://github.com/pydata/xarray/pull/3285#issuecomment-547837799 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0NzgzNzc5OQ== takluyver 327925 2019-10-30T10:37:23Z 2019-10-30T10:37:23Z MEMBER

OK, I've xfailed it.

 
{
    "total_count": 1,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 1,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
541307157 https://github.com/pydata/xarray/pull/3285#issuecomment-541307157 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0MTMwNzE1Nw== pep8speaks 24736507 2019-10-12T09:36:03Z 2019-10-30T10:01:17Z NONE

Hello @takluyver! 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-10-30 10:01:16 UTC
 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
547651559 https://github.com/pydata/xarray/pull/3285#issuecomment-547651559 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0NzY1MTU1OQ== shoyer 1217238 2019-10-29T22:09:10Z 2019-10-29T22:09:10Z MEMBER

In my experience it's better to open an issue, add an xfail decorator to the test, and merge the tests PR. Otherwise the initial PR can take a very long time and no other property-based tests get added.

+1 let's do that!

 
{
    "total_count": 1,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 1,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
547646808 https://github.com/pydata/xarray/pull/3285#issuecomment-547646808 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0NzY0NjgwOA== Zac-HD 12229877 2019-10-29T21:54:35Z 2019-10-29T21:56:21Z CONTRIBUTOR

OK, looks like the test failure now is real. Let me know if you want me to comment out the relevant line so the tests pass.

If we want to merge a subset of the tests then that's fine. Ofc even better if we can use these tests to find & fix the errors

In my experience it's better to open an issue, add an xfail decorator to the test, and merge the tests PR. Otherwise the initial PR can take a very long time and no other property-based tests get added.

In this case I'd duplicate the test, so there's one which does not allow empty dataframes and one (xfailing) which does.

It's also likely that the person who found the bug is not the best person to fix it, and requiring that they do so in order to merge a useful test just disincentives testing! 

{
    "total_count": 1,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 1,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
547588922 https://github.com/pydata/xarray/pull/3285#issuecomment-547588922 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0NzU4ODkyMg== max-sixty 5635139 2019-10-29T19:23:07Z 2019-10-29T19:23:07Z MEMBER

OK, looks like the test failure now is real. Let me know if you want me to comment out the relevant line so the tests pass.

If we want to merge a subset of the tests then that's fine. Ofc even better if we can use these tests to find & fix the errors

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
547587564 https://github.com/pydata/xarray/pull/3285#issuecomment-547587564 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0NzU4NzU2NA== max-sixty 5635139 2019-10-29T19:19:28Z 2019-10-29T19:19:28Z MEMBER

You're right @takluyver

It looks like hypothesis tests are running in the normal test suites. Anyone know offhand why that is? e.g. https://dev.azure.com/xarray/xarray/_build/results?buildId=1284

(that doesn't solve the test failure, though)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
547503432 https://github.com/pydata/xarray/pull/3285#issuecomment-547503432 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0NzUwMzQzMg== takluyver 327925 2019-10-29T16:13:52Z 2019-10-29T16:13:52Z MEMBER

OK, looks like the test failure now is real. Let me know if you want me to comment out the relevant line so the tests pass.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
547458156 https://github.com/pydata/xarray/pull/3285#issuecomment-547458156 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0NzQ1ODE1Ng== takluyver 327925 2019-10-29T14:48:52Z 2019-10-29T14:48:52Z MEMBER

Nope. I don't understand the error, though it looks like astropy has had something similar: https://github.com/astropy/astropy/issues/6424

Also black is now failing on a number of files not affected here.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
547453601 https://github.com/pydata/xarray/pull/3285#issuecomment-547453601 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0NzQ1MzYwMQ== takluyver 327925 2019-10-29T14:40:05Z 2019-10-29T14:40:05Z MEMBER

Merged master, crossing fingers that fixes it.

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
547451344 https://github.com/pydata/xarray/pull/3285#issuecomment-547451344 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0NzQ1MTM0NA== max-sixty 5635139 2019-10-29T14:35:37Z 2019-10-29T14:35:37Z MEMBER

This seems so close—could we fix the test (maybe that's just a merging of master?) and merge?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
540987510 https://github.com/pydata/xarray/pull/3285#issuecomment-540987510 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDU0MDk4NzUxMA== takluyver 327925 2019-10-11T09:20:16Z 2019-10-11T09:20:16Z MEMBER

As in my other PR, one suggested addition causes a test failure, and I've put that in the last commit.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894
529197447 https://github.com/pydata/xarray/pull/3285#issuecomment-529197447 https://api.github.com/repos/pydata/xarray/issues/3285 MDEyOklzc3VlQ29tbWVudDUyOTE5NzQ0Nw== max-sixty 5635139 2019-09-08T12:20:34Z 2019-09-08T12:20:34Z MEMBER

This looks great! I'll let someone who knows hypothesis better do a full review. Thanks for submitting @takluyver !

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis tests for roundtrip to & from pandas 490316894

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