home / github

Menu
  • GraphQL API
  • Search all tables

issue_comments

Table actions
  • GraphQL API for issue_comments

13 rows where issue = 1200581329 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 5

  • jhamman 4
  • shoyer 3
  • grlee77 3
  • dcherian 2
  • joshmoore 1

author_association 3

  • MEMBER 9
  • CONTRIBUTOR 3
  • NONE 1

issue 1

  • implement Zarr v3 spec support · 13 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1328155507 https://github.com/pydata/xarray/pull/6475#issuecomment-1328155507 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85PKgtz dcherian 2448579 2022-11-27T02:22:37Z 2022-11-27T02:22:37Z MEMBER

Thanks @grlee77 and @jhamman !

{
    "total_count": 1,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 1,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329
1320701325 https://github.com/pydata/xarray/pull/6475#issuecomment-1320701325 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85OuE2N jhamman 2443309 2022-11-19T00:31:47Z 2022-11-19T00:31:47Z MEMBER

This is ready to merge once https://github.com/pydata/xarray/pull/7300 is in.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329
1320632540 https://github.com/pydata/xarray/pull/6475#issuecomment-1320632540 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85Ot0Dc dcherian 2448579 2022-11-18T23:13:29Z 2022-11-18T23:13:47Z MEMBER

RTD failure is real.

/home/docs/checkouts/readthedocs.org/user_builds/xray/checkouts/6475/doc/whats-new.rst:28: WARNING: Duplicate explicit target name: "whats-new.2022.11.1". Otherwise, is this ready to merge?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329
1304558730 https://github.com/pydata/xarray/pull/6475#issuecomment-1304558730 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85NwfyK jhamman 2443309 2022-11-05T14:38:18Z 2022-11-05T14:38:18Z MEMBER

@grlee77, @rabernat, @joshmoore, and others - I think this is ready to review and/or merge. The Zarr-V3 tests are active in the CI Upstream / upstream-dev GitHub Action. The test failure on readthedocs is unrelated to this PR.

{
    "total_count": 1,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 1,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329
1254062705 https://github.com/pydata/xarray/pull/6475#issuecomment-1254062705 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85Kv3px grlee77 6528957 2022-09-21T18:15:26Z 2022-10-29T02:45:24Z CONTRIBUTOR

sorry about the long delay here. This has been updated for the V3 store paths used in Zarr >v2.12 and to remove the need for specifying path with v3 stores.

To do: - [x] wait for zarr v2.13 release, hopefully also including a new fix in #https://github.com/zarr-developers/zarr-python/pull/1142 - [x] update at least one CI test case to run the tests with zarr v2.13 and ZARR_V3_EXPERIMENTAL_API=1 enviroment variable

A separate issue is that consolidated metadata isn't in the core Zarr v3 spec, so we will need to have a Zarr Enhancement Proposal to formally define how the metadata should be stored. In the experimental API, it behaves as for v2 and is stored at /meta/root/consolidated by default.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329
1294266012 https://github.com/pydata/xarray/pull/6475#issuecomment-1294266012 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85NJO6c grlee77 6528957 2022-10-28T00:33:24Z 2022-10-28T00:33:24Z CONTRIBUTOR

I am happy for someone to take over if possible. Thank you.

{
    "total_count": 2,
    "+1": 2,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329
1294086466 https://github.com/pydata/xarray/pull/6475#issuecomment-1294086466 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85NIjFC jhamman 2443309 2022-10-27T21:32:42Z 2022-10-27T21:32:42Z MEMBER

@grlee77 - I'm curious if you are planning to return to this PR or if it would be helpful if someone brought it to completion?

{
    "total_count": 1,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 1,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329
1281657040 https://github.com/pydata/xarray/pull/6475#issuecomment-1281657040 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85MZIjQ jhamman 2443309 2022-10-18T00:23:20Z 2022-10-18T00:23:20Z MEMBER

A separate issue is that consolidated metadata isn't in the core Zarr v3 spec, so we will need to have a Zarr Enhancement Proposal to formally define how the metadata should be stored. In the experimental API, it behaves as for v2 and is stored at /meta/root/consolidated by default.

I think it would be fine to disallow consolidated metadata for v3 until there is a spec in place. This is going to be experimental for some time so I don't see the harm in raising an error when consolidated=True and version=3. I think this is better than guessing what the v3 extension will specify.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329
1257908330 https://github.com/pydata/xarray/pull/6475#issuecomment-1257908330 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85K-ihq joshmoore 88113 2022-09-26T11:46:05Z 2022-09-26T11:46:05Z NONE

wait for zarr v2.13 release,

Done. And should be out on conda-forge later today.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329
1137661171 https://github.com/pydata/xarray/pull/6475#issuecomment-1137661171 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85Dz1Tz shoyer 1217238 2022-05-25T18:10:21Z 2022-05-25T18:10:21Z MEMBER

One issue with relying only on Array and Group as currently implemented in Zarr-Python is that we can create array nodes outside of any group subfolder. e.g. one can currently create an Array directly at path 'array1' and this would put the chunks under 'data/root/array1/', and metadata at 'meta/root/array1.array.json'. However, the root itself is not a Group. A group is basically a subfolder under root (e.g.' open_group with path = group1 creates '/meta/root/group1/' folder and 'meta/root/group1.group.json' metadata). There is no mechanism in the spec to open root directly as a Group!

is there an issue on the Zarr side where this is currently being discussed?

I opened up https://github.com/zarr-developers/zarr-python/issues/1039

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329
1099307673 https://github.com/pydata/xarray/pull/6475#issuecomment-1099307673 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85BhhqZ shoyer 1217238 2022-04-14T15:33:54Z 2022-04-14T15:33:54Z MEMBER

One issue with relying only on Array and Group as currently implemented in Zarr-Python is that we can create array nodes outside of any group subfolder. e.g. one can currently create an Array directly at path 'array1' and this would put the chunks under 'data/root/array1/', and metadata at 'meta/root/array1.array.json'. However, the root itself is not a Group. A group is basically a subfolder under root (e.g.' open_group with path = group1 creates '/meta/root/group1/' folder and 'meta/root/group1.group.json' metadata). There is no mechanism in the spec to open root directly as a Group!

is there an issue on the Zarr side where this is currently being discussed?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329
1099265057 https://github.com/pydata/xarray/pull/6475#issuecomment-1099265057 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85BhXQh grlee77 6528957 2022-04-14T14:48:39Z 2022-04-14T14:55:07Z CONTRIBUTOR

Does Zarr v3 have a notion of a "root" group? That feels like a more sensible default to me, both for Xarray and Zarr-Python

I think we likely need to introduce a separate Hierarchy class as in the early zarrita python prototype and the xtensor-zarr C++ implementation to be able to access the root via public API. The concept of "hierarchy" as a collection of Nodes which are either Arrays or Groups is mentioned in the spec, but there is no corresponding class for this in zarr-python currently.

One issue with relying only on Array and Group as currently implemented in Zarr-Python is that we can create array nodes outside of any group subfolder. e.g. one can currently create an Array directly at path 'array1' and this would put the chunks under 'data/root/array1/', and metadata at 'meta/root/array1.array.json'. However, the root itself is not a Group. A group is basically a subfolder under root (e.g.' open_group with path = group1 creates '/meta/root/group1/' folder and 'meta/root/group1.group.json' metadata). There is no mechanism in the spec to open root directly as a Group!

This sounds fine for now, but I am concerned that it will slow the adoption of Zarr v3. Eventually, we would presumably want to change the default to version 3, but this is difficult to do if it entirely breaks backwards compatibility.

We did define DEFAULT_ZARR_VERSION=2 (privately). If we update that variable to 3 in a future release, the default when zarr_version is not specified will change.

My preference would be for the default behavior to try opening Zarr v2, and fall back to opening in v3 mode, even if this requires attempting to open a file from the store. This is similar to how Xarray handles other Zarr versioning issues (e.g., for consolidated metadata). Perhaps Zarr-Python could raise an informative error that we could catch if the Zarr version is incorrect, or even handle this behavior itself?

Yeah, something like this seems feasible on the Zarr side for convenience routines like open_group. The v3 spec requires zarr.json metadata that specifies the protocol version. If we traverse up the directory tree and do not find this file, then it is not a valid v3 or later spec and we can try opening it as v2.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329
1098229361 https://github.com/pydata/xarray/pull/6475#issuecomment-1098229361 https://api.github.com/repos/pydata/xarray/issues/6475 IC_kwDOAMm_X85BdaZx shoyer 1217238 2022-04-13T16:04:23Z 2022-04-13T16:04:23Z MEMBER
  • The v3 spec requires a path be specified when calling open_group or open_consolidated. This PR currently just sets a default group name of 'xarray' if one is not specified via the group kwarg to ZarrStore.open_group. I think that is convenient, but one could instead be stricter and raise an error in this case.

Does Zarr v3 have a notion of a "root" group? That feels like a more sensible default to me, both for Xarray and Zarr-Python

  • If a string corresponding to a filesystem path or URL is used for store, then it is not possible to infer which version of the zarr spec is desired. In this case, the user must specify zarr_version to choose the zarr protocol version. The default of zarr_version=None will infer the version from a zarr BaseStore subclass when possible, otherwise defaulting to zarr_version=2 for backwards compatibility.

This sounds fine for now, but I am concerned that it will slow the adoption of Zarr v3. Eventually, we would presumably want to change the default to version 3, but this is difficult to do if it entirely breaks backwards compatibility.

My preference would be for the default behavior to try opening Zarr v2, and fall back to opening in v3 mode, even if this requires attempting to open a file from the store. This is similar to how Xarray handles other Zarr versioning issues (e.g., for consolidated metadata). Perhaps Zarr-Python could raise an informative error that we could catch if the Zarr version is incorrect, or even handle this behavior itself?

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  implement Zarr v3 spec support 1200581329

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 15.761ms · About: xarray-datasette