home / github

Menu
  • GraphQL API
  • Search all tables

issue_comments

Table actions
  • GraphQL API for issue_comments

10 rows where issue = 958878760 sorted by updated_at descending

✖
✖

✎ View and edit SQL

This data as json, CSV (advanced)

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

user 5

  • Illviljan 5
  • max-sixty 2
  • pep8speaks 1
  • github-actions[bot] 1
  • headtr1ck 1

author_association 4

  • MEMBER 7
  • COLLABORATOR 1
  • CONTRIBUTOR 1
  • NONE 1

issue 1

  • Allow .attrs to support any dict-likes · 10 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1276551797 https://github.com/pydata/xarray/pull/5667#issuecomment-1276551797 https://api.github.com/repos/pydata/xarray/issues/5667 IC_kwDOAMm_X85MFqJ1 headtr1ck 43316012 2022-10-12T18:08:24Z 2022-10-12T18:08:24Z COLLABORATOR

If that is still an open issue we could merge current main, try to fix the resulting typing problems.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Allow .attrs to support any dict-likes 958878760
891658579 https://github.com/pydata/xarray/pull/5667#issuecomment-891658579 https://api.github.com/repos/pydata/xarray/issues/5667 IC_kwDOAMm_X841JaFT github-actions[bot] 41898282 2021-08-03T08:45:35Z 2021-10-31T10:23:15Z CONTRIBUTOR

Unit Test Results

6 files           6 suites   56m 18s :stopwatch: 16 290 tests 14 550 :heavy_check_mark: 1 739 :zzz: 1 :x: 90 936 runs  82 737 :heavy_check_mark: 8 198 :zzz: 1 :x:

For more details on these failures, see this check.

Results for commit 650ce229.

:recycle: This comment has been updated with latest results.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Allow .attrs to support any dict-likes 958878760
891656976 https://github.com/pydata/xarray/pull/5667#issuecomment-891656976 https://api.github.com/repos/pydata/xarray/issues/5667 IC_kwDOAMm_X841JZsQ pep8speaks 24736507 2021-08-03T08:43:24Z 2021-10-31T09:57:53Z NONE

Hello @Illviljan! 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 2021-10-31 09:57:53 UTC
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Allow .attrs to support any dict-likes 958878760
902482335 https://github.com/pydata/xarray/pull/5667#issuecomment-902482335 https://api.github.com/repos/pydata/xarray/issues/5667 IC_kwDOAMm_X841ysmf Illviljan 14371165 2021-08-20T07:06:19Z 2021-08-20T07:10:16Z MEMBER

Here's further tests to check how fast different class checkers are:

```python from typing import MutableMapping

class Test2(MutableMapping): def init(self, args, kwargs): self.data = dict(args, **kwargs)

def __getitem__(self, key):
    pass

def __setitem__(self, key, value):
    pass

def __delitem__(self, key):
    pass

def __iter__(self):
    pass

def __len__(self):
    pass

b = Test2()

%timeit issubclass(type(b), MutableMapping) 711 ns ± 5.33 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

%timeit isinstance(b, MutableMapping) 853 ns ± 6.29 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

If you want to get really fast you can check for one of the required attributes MutableMapping has

%timeit hasattr(b, "update") 82.6 ns ± 0.181 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each) ```

isinstance is rather slow as can be seen. Considering just doing dict(b) takes about 200ns which is basically what the original implementation was it doesn't feel that good to add a check that adds 800ns of wait time.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Allow .attrs to support any dict-likes 958878760
902468479 https://github.com/pydata/xarray/pull/5667#issuecomment-902468479 https://api.github.com/repos/pydata/xarray/issues/5667 IC_kwDOAMm_X841ypN_ Illviljan 14371165 2021-08-20T06:38:57Z 2021-08-20T06:38:57Z MEMBER

Is python/mypy#3004 still an issue?

pre-commit suggests it's here: https://github.com/pydata/xarray/pull/5667/files#diff-3c0ce7941684cbac55c00ab890684f86acc1de1908ee2afa915dbcb7c944105aR100 — but I guess there's some reason we can't only accept a MutableMapping in that function?

Yes, it is still an issue. I've cheated though and used type: ignore on a few places, that's why its been passing the checks.

Mappings (in the form of FrozenDict) are used to initialize .attrs in xr.open_dataset for example. So we can't unfortunately accept MutableMappings only.

Does pyright handle properties with setters and getters?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Allow .attrs to support any dict-likes 958878760
902291778 https://github.com/pydata/xarray/pull/5667#issuecomment-902291778 https://api.github.com/repos/pydata/xarray/issues/5667 IC_kwDOAMm_X841x-FC max-sixty 5635139 2021-08-19T22:23:49Z 2021-08-19T22:23:49Z MEMBER

Is python/mypy#3004 still an issue?

pre-commit suggests it's here: https://github.com/pydata/xarray/pull/5667/files#diff-3c0ce7941684cbac55c00ab890684f86acc1de1908ee2afa915dbcb7c944105aR100 — but I guess there's some reason we can't only accept a MutableMapping in that function?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Allow .attrs to support any dict-likes 958878760
894636258 https://github.com/pydata/xarray/pull/5667#issuecomment-894636258 https://api.github.com/repos/pydata/xarray/issues/5667 IC_kwDOAMm_X841UxDi Illviljan 14371165 2021-08-07T10:26:55Z 2021-08-07T10:26:55Z MEMBER

Think I'm running into https://github.com/python/mypy/issues/3004. Not completely sure why this worked before though.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Allow .attrs to support any dict-likes 958878760
894461697 https://github.com/pydata/xarray/pull/5667#issuecomment-894461697 https://api.github.com/repos/pydata/xarray/issues/5667 IC_kwDOAMm_X841UGcB Illviljan 14371165 2021-08-06T19:06:12Z 2021-08-06T19:06:12Z MEMBER

I'm surprised about the deepcopy being so slow too, I thought it would be similar in speed in this case and just increase if dealing with mutable objects.

But using .copy is 100% compatible with how attrs has behaved before. So we come back to the question what type attrs should be? The options I see is going with dict or MutableMapping.

I'm starting to lean towards mutablemapping because subclassing dict has been rather difficult compared to mutablemapping.

And if we go with mutablemapping then we should use copy.copy.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Allow .attrs to support any dict-likes 958878760
894378303 https://github.com/pydata/xarray/pull/5667#issuecomment-894378303 https://api.github.com/repos/pydata/xarray/issues/5667 IC_kwDOAMm_X841TyE_ max-sixty 5635139 2021-08-06T16:35:33Z 2021-08-06T16:35:33Z MEMBER

Using a.copy() seems to be the way to go if you want to do a shallow copy of a dict.

That is an interesting result (even aside from the main result here, I'm not sure what python is doing such that copy.copy(a) has different performance from copy(a)!)

But python is slow, and nanos are short — unless there's a noticeable impact on the overall performance, then prioritizing flexibility and compatibility are more consistent with the goals of the library. Does that make sense?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Allow .attrs to support any dict-likes 958878760
893208691 https://github.com/pydata/xarray/pull/5667#issuecomment-893208691 https://api.github.com/repos/pydata/xarray/issues/5667 IC_kwDOAMm_X841PUhz Illviljan 14371165 2021-08-05T06:45:09Z 2021-08-05T07:07:29Z MEMBER

Some fun performance comparisons related to copying and initializing dicts:

```python a = dict(a=2, b=3)

%timeit dict(a) 207 ns ± 3.41 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

%timeit a.copy() 82.6 ns ± 0.425 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

import copy %timeit copy.copy(a) 313 ns ± 3.59 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

from copy import copy %timeit copy(a) 290 ns ± 3.63 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

from copy import deepcopy %timeit deepcopy(a) 3.39 µs ± 55.6 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each) ```

Using a.copy() seems to be the way to go if you want to do a shallow copy of a dict.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Allow .attrs to support any dict-likes 958878760

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]);
Powered by Datasette · Queries took 24.042ms · About: xarray-datasette
  • Sort ascending
  • Sort descending
  • Facet by this
  • Hide this column
  • Show all columns
  • Show not-blank rows