issue_comments

Table actions

10 rows where issue = 151483622 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

user 3

author_association 2

issue 1

  • Fixed a bug that showed when trying to establish equivalence of two a… · 10 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
228419690 https://github.com/pydata/xarray/pull/837#issuecomment-228419690 https://api.github.com/repos/pydata/xarray/issues/837 MDEyOklzc3VlQ29tbWVudDIyODQxOTY5MA== shoyer 1217238 2016-06-24T18:13:36Z 2016-06-24T18:13:36Z MEMBER 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Fixed a bug that showed when trying to establish equivalence of two a… 151483622
222873697 https://github.com/pydata/xarray/pull/837#issuecomment-222873697 https://api.github.com/repos/pydata/xarray/issues/837 MDEyOklzc3VlQ29tbWVudDIyMjg3MzY5Nw== shoyer 1217238 2016-06-01T02:07:42Z 2016-06-01T02:07:42Z MEMBER

This looks good to me. The test could probably still be made more succinct but this covers the essentials. I like that it provides an integration test for using rec arrays.

@jhamman any more comments before I merge?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Fixed a bug that showed when trying to establish equivalence of two a… 151483622
222017597 https://github.com/pydata/xarray/pull/837#issuecomment-222017597 https://api.github.com/repos/pydata/xarray/issues/837 MDEyOklzc3VlQ29tbWVudDIyMjAxNzU5Nw== maciekswat 5384661 2016-05-26T22:53:00Z 2016-05-26T22:53:00Z CONTRIBUTOR

Added bugfix entry in what's new

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Fixed a bug that showed when trying to establish equivalence of two a… 151483622
218359954 https://github.com/pydata/xarray/pull/837#issuecomment-218359954 https://api.github.com/repos/pydata/xarray/issues/837 MDEyOklzc3VlQ29tbWVudDIxODM1OTk1NA== jhamman 2443309 2016-05-11T04:38:51Z 2016-05-11T04:38:51Z MEMBER

@maciekswat - this is almost there. @shoyer and I each have comments remaining that should be addressed and then we need a bug fix entry in "what's new". Thanks!

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Fixed a bug that showed when trying to establish equivalence of two a… 151483622
216556267 https://github.com/pydata/xarray/pull/837#issuecomment-216556267 https://api.github.com/repos/pydata/xarray/issues/837 MDEyOklzc3VlQ29tbWVudDIxNjU1NjI2Nw== maciekswat 5384661 2016-05-03T15:00:17Z 2016-05-03T15:00:17Z CONTRIBUTOR

Added a comment explaining why we need to catch typeError in array_equiv

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Fixed a bug that showed when trying to establish equivalence of two a… 151483622
216338679 https://github.com/pydata/xarray/pull/837#issuecomment-216338679 https://api.github.com/repos/pydata/xarray/issues/837 MDEyOklzc3VlQ29tbWVudDIxNjMzODY3OQ== maciekswat 5384661 2016-05-02T19:35:04Z 2016-05-02T19:35:04Z CONTRIBUTOR

I simplified the test - please feel free to do further edits, also added doc string explaining what the test checks for - essentially it checks if DataArray subtraction behaves correctly when dimension is a structured numpy array. 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Fixed a bug that showed when trying to establish equivalence of two a… 151483622
216313208 https://github.com/pydata/xarray/pull/837#issuecomment-216313208 https://api.github.com/repos/pydata/xarray/issues/837 MDEyOklzc3VlQ29tbWVudDIxNjMxMzIwOA== maciekswat 5384661 2016-05-02T18:06:38Z 2016-05-02T18:06:38Z CONTRIBUTOR

just fixed the formatting according to PEP8 guidelines

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Fixed a bug that showed when trying to establish equivalence of two a… 151483622
216310274 https://github.com/pydata/xarray/pull/837#issuecomment-216310274 https://api.github.com/repos/pydata/xarray/issues/837 MDEyOklzc3VlQ29tbWVudDIxNjMxMDI3NA== shoyer 1217238 2016-05-02T17:56:03Z 2016-05-02T17:56:03Z MEMBER

@maciekswat awesome -- thanks!

can you please run your code through a PEP8 checker like pep8 or pyflakes? some of your spacing is off.

also, please add a note to "What's new" giving yourself credit for the bug fix :).

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Fixed a bug that showed when trying to establish equivalence of two a… 151483622
216307904 https://github.com/pydata/xarray/pull/837#issuecomment-216307904 https://api.github.com/repos/pydata/xarray/issues/837 MDEyOklzc3VlQ29tbWVudDIxNjMwNzkwNA== maciekswat 5384661 2016-05-02T17:48:02Z 2016-05-02T17:48:02Z CONTRIBUTOR

I have rewrote array_equiv fcn from ops.py to use single comparison (the one which tests from null is inside try/except clause). Also added test case to test_DataArray.py

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Fixed a bug that showed when trying to establish equivalence of two a… 151483622
215291343 https://github.com/pydata/xarray/pull/837#issuecomment-215291343 https://api.github.com/repos/pydata/xarray/issues/837 MDEyOklzc3VlQ29tbWVudDIxNTI5MTM0Mw== shoyer 1217238 2016-04-28T02:36:01Z 2016-04-28T02:36:01Z MEMBER

Thanks for looking into this!

Just to summarize, the issue is that isnull doesn't work on structured dtypes:

```

pd.isnull(p_data) TypeError: Not implemented for this type ```

The problem with your current patch is that it will actually compare all array elements twice. Because checks for equality are done quite often in xarray, this will have prohibitive performance cost.

So, we'll either need to check for structured dtypes, or catch TypeError exceptions and try doing the comparison without checking for nulls.

Also, we need tests, to verify that the bug is fixed.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Fixed a bug that showed when trying to establish equivalence of two a… 151483622

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