issue_comments

Table actions

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

✎ View and edit SQL

This data as json, CSV (advanced)

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

user 1

  • keewis · 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
554370601 https://github.com/pydata/xarray/pull/3493#issuecomment-554370601 https://api.github.com/repos/pydata/xarray/issues/3493 MDEyOklzc3VlQ29tbWVudDU1NDM3MDYwMQ== keewis 14808389 2019-11-15T14:04:41Z 2019-11-15T14:49:32Z MEMBER

ahh, now this python cls = staticmethod(lambda *args: Variable(*args).chunk()) makes a lot more sense, I didn't understand the purpose of VariableSubclassobjects before. Yes, that may potentially save us some code. I'll try it out and submit a PR for Variable

We didn't go back & forth re fixtures yet. I'll also investigate the overlap, but I think it's only partial.

 
{
    "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
554113722 https://github.com/pydata/xarray/pull/3493#issuecomment-554113722 https://api.github.com/repos/pydata/xarray/issues/3493 MDEyOklzc3VlQ29tbWVudDU1NDExMzcyMg== keewis 14808389 2019-11-14T22:36:51Z 2019-11-14T22:36:51Z MEMBER

hmm... well, I certainly agree the tests are often quite verbose (maybe too verbose) and sometimes also test functionality of pint (e.g. when incompatible, compatible and identical units are tried). I didn't check, but I don't remember any overlaps with tests from test_dataarray.py or test_dataset.py (if that's what you meant).

To reduce the code, it might be worth to only test compatible units. 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.

If Variable is part of the external API it definitely needs tests.

Reusing tests from test_dataarray.py / test_dataset.py / test_variable.py is tempting, but I don't think it is possible unless we rewrite them. Am I missing something?

 
{
    "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
553996209 https://github.com/pydata/xarray/pull/3493#issuecomment-553996209 https://api.github.com/repos/pydata/xarray/issues/3493 MDEyOklzc3VlQ29tbWVudDU1Mzk5NjIwOQ== keewis 14808389 2019-11-14T17:36:14Z 2019-11-14T17:37:10Z MEMBER

thanks for reviewing, @max-sixty

I did not plan to add tests for Variable, but that might be an oversight on my part: I have been writing tests for the functions / methods in api.rst where Variable is listed as part of the Advanced API, which means I ignored it. Also, if my understanding of xarray's internals is correct, Variable is used in both DataArray and Dataset operations, so the tests on these should implicitly also test Variable. Considering that implicit is usually not a good idea, should I add tests for Variable?

I don't think inheritance will help much, but that example could definitely be used as a reference / inspiration.

 
{
    "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
552154627 https://github.com/pydata/xarray/pull/3493#issuecomment-552154627 https://api.github.com/repos/pydata/xarray/issues/3493 MDEyOklzc3VlQ29tbWVudDU1MjE1NDYyNw== keewis 14808389 2019-11-10T01:54:38Z 2019-11-10T13:48:51Z MEMBER

this should be ready for review and merge, unless anyone wants tests for auto_combine or map_blocks? I skipped auto_combine because it is listed as deprecated, and map_blocks because until now I avoided testing pint+dask (if I understand it correctly, map_blocks without dask is apply_ufunc?).

I will add tests for Dataset.broadcast_like and DataArray.broadcast_like in a new PR and then go over all tests up until now to fix any style inconsistencies and obvious errors. After that, I guess we have to wait on pint?

Edit: the failing distributed test on MacOSX as well as the timeout for the docs should be unrelated, I think

 
{
    "total_count": 1,
    "+1": 1,
    "-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]);