issue_comments

Table actions

4 rows where issue = 519490511 and user = 5635139 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

user 1

  • max-sixty · 4 ✖

issue 1

  • Tests for module-level functions with units · 4 ✖

author_association 1

id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
554222017 https://github.com/pydata/xarray/pull/3493#issuecomment-554222017 https://api.github.com/repos/pydata/xarray/issues/3493 MDEyOklzc3VlQ29tbWVudDU1NDIyMjAxNw== max-sixty 5635139 2019-11-15T05:46:14Z 2019-11-15T05:46:14Z MEMBER

Overall the tests are great, and the breadth of coverage is impressive. That's more important than their form!

The way I was thinking about leveraging existing tests is that there are a) some tests that test existing functions at least run on pint-backed arrays and b) some tests that test whether the units work correctly when used in xarray

Any opportunities to use existing code would be on (a). In the above linked Variable tests, we re-run all the tests for a dask-backed Variable by inheriting from the test class, and xfail those that don't work. (though sounds like you think that wouldn't work in this case?)

We could also try to use helper functions for data creation, but while that reduces the code it also makes understanding it a little bit harder.

Yes, there's some repetition. Did we go back & forth before re putting some of the duplicated setup in fixtures? That could cut down some boilerplate if there's a lot of overlap (though if there's only partial overlap, also increase complication, as you point out)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Tests for module-level functions with units 519490511
554034633 https://github.com/pydata/xarray/pull/3493#issuecomment-554034633 https://api.github.com/repos/pydata/xarray/issues/3493 MDEyOklzc3VlQ29tbWVudDU1NDAzNDYzMw== max-sixty 5635139 2019-11-14T19:09:55Z 2019-11-14T19:09:55Z MEMBER

I was predominately suggesting that as a way of saving your time & code on the margin (test_units.py is 4330 LOC!), and it seems like there's some overlap in code that's testing whether functions work at all, before whether the units are working correctly (though agree there's a Variable / DataArray distinction).

As from any time or code savings, I think that it's not strictly necessary to test Variable separately from Dataset & DataArray—it is implicit but it's also the external API—what do others think?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Tests for module-level functions with units 519490511
553681522 https://github.com/pydata/xarray/pull/3493#issuecomment-553681522 https://api.github.com/repos/pydata/xarray/issues/3493 MDEyOklzc3VlQ29tbWVudDU1MzY4MTUyMg== max-sixty 5635139 2019-11-14T01:28:07Z 2019-11-14T01:28:07Z MEMBER

While it won't cover all the use cases, check out https://github.com/pydata/xarray/blob/master/xarray/tests/test_variable.py#L1819 when you get a chance; it's possible that inheriting from that test with a pint array might give you some tests for free

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Tests for module-level functions with units 519490511
553680619 https://github.com/pydata/xarray/pull/3493#issuecomment-553680619 https://api.github.com/repos/pydata/xarray/issues/3493 MDEyOklzc3VlQ29tbWVudDU1MzY4MDYxOQ== max-sixty 5635139 2019-11-14T01:24:15Z 2019-11-14T01:24:15Z MEMBER

Thanks a lot as ever @keewis !

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Tests for module-level functions with units 519490511

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