issue_comments

Table actions

7 rows where issue = 1336119080 and user = 35968931 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

  • TomNicholas · 7 ✖

issue 1

  • Hypothesis strategies in xarray.testing.strategies · 7 ✖

author_association 1

id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1251142486 https://github.com/pydata/xarray/pull/6908#issuecomment-1251142486 https://api.github.com/repos/pydata/xarray/issues/6908 IC_kwDOAMm_X85KkutW TomNicholas 35968931 2022-09-19T14:58:29Z 2022-09-19T14:58:55Z MEMBER

@Zac-HD if I could request one more review please! The two remaining problems for me are:

1) How should we alter the API of datasets to make it easier to construct Dataset objects containing duck-typed arrays (see https://github.com/pydata/xarray/pull/6908#discussion_r974066789) 2) Why does the example generation performance seem to have gotten worse? :confused: I added what I thought were small refactors (e.g. _sizes_from_dim_names) which may somehow be related, but I'm not sure.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis strategies in xarray.testing.strategies 1336119080
1238321913 https://github.com/pydata/xarray/pull/6908#issuecomment-1238321913 https://api.github.com/repos/pydata/xarray/issues/6908 IC_kwDOAMm_X85Jz0r5 TomNicholas 35968931 2022-09-06T15:38:33Z 2022-09-06T15:38:33Z MEMBER

It might take me a little while to get through all this, but this is a great review, thanks again @Zac-HD !

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis strategies in xarray.testing.strategies 1336119080
1235820266 https://github.com/pydata/xarray/pull/6908#issuecomment-1235820266 https://api.github.com/repos/pydata/xarray/issues/6908 IC_kwDOAMm_X85JqR7q TomNicholas 35968931 2022-09-02T19:07:59Z 2022-09-02T19:07:59Z MEMBER

@Zac-HD I think this is ready for another review (or a least a reply to the couple of unresolved comments)!

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis strategies in xarray.testing.strategies 1336119080
1218358572 https://github.com/pydata/xarray/pull/6908#issuecomment-1218358572 https://api.github.com/repos/pydata/xarray/issues/6908 IC_kwDOAMm_X85Inq0s TomNicholas 35968931 2022-08-17T18:30:20Z 2022-08-17T18:30:20Z MEMBER

Thanks for the feedback @Zac-HD, that's extremely helpful already! Sounds like we need to decide on the general API approach, then I can go back and fix all the internals.

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis strategies in xarray.testing.strategies 1336119080
1217246890 https://github.com/pydata/xarray/pull/6908#issuecomment-1217246890 https://api.github.com/repos/pydata/xarray/issues/6908 IC_kwDOAMm_X85Ijbaq TomNicholas 35968931 2022-08-16T22:47:42Z 2022-08-16T22:47:42Z MEMBER

I'll aim for a proper review tonight!

Amazing! Thank you!

Quick remarks: strategies accepting strategies is fine, though our API style guide suggests accepting strategies xor values

Oh I hadn't seen that! Looks like I independently made similar API decisions though, as far as I can tell.

 
{
    "total_count": 2,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 1,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis strategies in xarray.testing.strategies 1336119080
1216748280 https://github.com/pydata/xarray/pull/6908#issuecomment-1216748280 https://api.github.com/repos/pydata/xarray/issues/6908 IC_kwDOAMm_X85Ihhr4 TomNicholas 35968931 2022-08-16T14:52:06Z 2022-08-16T14:52:06Z MEMBER

@Zac-HD would you be able to give your hypothesis-expert opinions on the general design of these strategies? Especially the whole "strategies that accept multiple other strategies" thing.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis strategies in xarray.testing.strategies 1336119080
1212153036 https://github.com/pydata/xarray/pull/6908#issuecomment-1212153036 https://api.github.com/repos/pydata/xarray/issues/6908 IC_kwDOAMm_X85IP_zM TomNicholas 35968931 2022-08-11T15:33:52Z 2022-08-11T15:33:52Z MEMBER

I also added my chunking strategy from https://github.com/HypothesisWorks/hypothesis/issues/3433

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Hypothesis strategies in xarray.testing.strategies 1336119080

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