home / github

Menu
  • Search all tables
  • GraphQL API

issue_comments

Table actions
  • GraphQL API for issue_comments

8 rows where author_association = "MEMBER" and issue = 1094725752 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 4

  • max-sixty 4
  • mathause 2
  • shoyer 1
  • TomNicholas 1

issue 1

  • dimensions: type as `str | Iterable[Hashable]`? · 8 ✖

author_association 1

  • MEMBER · 8 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1138997020 https://github.com/pydata/xarray/issues/6142#issuecomment-1138997020 https://api.github.com/repos/pydata/xarray/issues/6142 IC_kwDOAMm_X85D47cc max-sixty 5635139 2022-05-26T20:27:19Z 2022-05-26T20:27:19Z MEMBER

Ah, that's a good case @headtr1ck . So str | Iterable[Hashable] works when one is checked before the other, but not when they're required to be disjoint, like an overload.

I don't have an immediate solution in mind unfortunately. I'm not sure having different return types is a great design in general, though I can see the logic for it in this case...

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  dimensions: type as `str | Iterable[Hashable]`? 1094725752
1132117191 https://github.com/pydata/xarray/issues/6142#issuecomment-1132117191 https://api.github.com/repos/pydata/xarray/issues/6142 IC_kwDOAMm_X85DerzH max-sixty 5635139 2022-05-19T19:28:04Z 2022-05-19T19:28:04Z MEMBER

Only remotely related to this issue, but would it help to type the dimension (or variable names etc) Mappings with covariant Hashable keys to prevent the issue that {"a":1} is incompatible with Mapping[Hashable, Any]?

I spent a non-trivial amount of time on this a year ago or so — my understanding is that Mapping was not covariant in its keys, exactly as you point out.

But I'm also not sure we lose anything with Mapping[Any, foo] — the key needs to be Hashable anyway. I don't have a strong objection, but do we gain anything?

e.g. https://mypy-play.net/?mypy=latest&python=3.10&gist=a3ea9f6fce5a678803bd7563a54cd9ca

See https://mypy-play.net/?mypy=latest&python=3.10&gist=b2c27a5ca4aad9e8ce63cd74dc4032b0

Very cool! I didn't even know about this.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  dimensions: type as `str | Iterable[Hashable]`? 1094725752
1116791799 https://github.com/pydata/xarray/issues/6142#issuecomment-1116791799 https://api.github.com/repos/pydata/xarray/issues/6142 IC_kwDOAMm_X85CkOP3 max-sixty 5635139 2022-05-04T00:21:34Z 2022-05-04T00:21:34Z MEMBER

there are so many inconsistencies that I even wonder if it is possible to use anything but strings in "production".

I would have thought this is indeed the case. I think we're quite open to adjusting this so that it works, but it would probably needs some work, including defining common patterns — e.g. the title of this issue is a good approach

Apart from arguments randomly being called dim or dims

I also think we'd be up for reforming this! Generally this is the first arg and so only used positionally, but it still makes sense to have them consistent.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  dimensions: type as `str | Iterable[Hashable]`? 1094725752
1007746043 https://github.com/pydata/xarray/issues/6142#issuecomment-1007746043 https://api.github.com/repos/pydata/xarray/issues/6142 IC_kwDOAMm_X848EPv7 mathause 10194086 2022-01-07T21:15:23Z 2022-01-07T21:15:23Z MEMBER

I agree this is confusing, what is meant here is to use a tuple as the name for one dimension. An example:

python ds = xr.Dataset({"x": ([("a", "b")], [1])}) print(ds) print(f"{ds.x.dims = }")

```python <xarray.Dataset> Dimensions: (('a', 'b'): 1) Dimensions without coordinates: ('a', 'b') Data variables: x (('a', 'b')) int64 1

ds.x.dims = (('a', 'b'),) ```


I found the issue again where we discussed that we want to keep dim: Hashable: #4821

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  dimensions: type as `str | Iterable[Hashable]`? 1094725752
1007729786 https://github.com/pydata/xarray/issues/6142#issuecomment-1007729786 https://api.github.com/repos/pydata/xarray/issues/6142 IC_kwDOAMm_X848ELx6 TomNicholas 35968931 2022-01-07T20:46:40Z 2022-01-07T20:46:40Z MEMBER

We need to be careful typing functions where only one dim is allowed, e.g. xr.concat, which should probably set dim: Hashable (and make sure it works).

Sorry, I don't understand this - if I want to type hint concat so that no-one can pass more than one dim, isn't my only option to type hint it as str? Because if I type hint it as Hashable they could pass ("x", "y"), which is two dims? Maybe I'm not following properly...

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  dimensions: type as `str | Iterable[Hashable]`? 1094725752
1007250588 https://github.com/pydata/xarray/issues/6142#issuecomment-1007250588 https://api.github.com/repos/pydata/xarray/issues/6142 IC_kwDOAMm_X848CWyc mathause 10194086 2022-01-07T09:13:21Z 2022-01-07T09:13:21Z MEMBER

This will still work as two dims, since having str | Iterable[Hashable] rather than Hashable | Iterable[Hashable] means it will fail the first check. So if someone really wants a single dim as ("x", "y"), they can pass (("x", "y"),).

Yes, I think that is the gist of the proposal. I also think that it is quite elegant.

Does this mean that this would be confusion free?

I think it would be relatively unambiguous but not necessarily entirely confusion free for the user ("Why is the type of a single dim more restricted than of several dimensions?"). Maybe this warrants an entry in our FAQ or somewhere? Also we need to go over our code and update our typing and make sure stuff like xr.DataArray([1], dims=[("x", "y")] works as proposed here.

{
    "total_count": 2,
    "+1": 2,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  dimensions: type as `str | Iterable[Hashable]`? 1094725752
1006622944 https://github.com/pydata/xarray/issues/6142#issuecomment-1006622944 https://api.github.com/repos/pydata/xarray/issues/6142 IC_kwDOAMm_X847_9jg max-sixty 5635139 2022-01-06T14:13:58Z 2022-01-06T14:13:58Z MEMBER

Thanks for writing this out. I realize something clever about the proposal:

Our very first line of code in the docs passes dims as a tuple dims=("x", "y"): https://xarray.pydata.org/en/stable/getting-started-guide/quick-overview.html#create-a-dataarray

This will still work as two dims, since having str | Iterable[Hashable] rather than Hashable | Iterable[Hashable] means it will fail the first check. So if someone really wants a single dim as ("x", "y"), they can pass (("x", "y"),).

Does this mean that this would be confusion free?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  dimensions: type as `str | Iterable[Hashable]`? 1094725752
1006104672 https://github.com/pydata/xarray/issues/6142#issuecomment-1006104672 https://api.github.com/repos/pydata/xarray/issues/6142 IC_kwDOAMm_X8479_Bg shoyer 1217238 2022-01-05T21:48:05Z 2022-01-05T21:48:05Z MEMBER

@shoyer - did I get this right from the discussion?

Yes, this was my suggestion.

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  dimensions: type as `str | Iterable[Hashable]`? 1094725752

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 1445.08ms · About: xarray-datasette
  • Sort ascending
  • Sort descending
  • Facet by this
  • Hide this column
  • Show all columns
  • Show not-blank rows