issue_comments

Table actions

8 rows where issue = 1352315409 and user = 30196 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

  • ianliu · 8 ✖

issue 1

  • Expose `memory` argument for "netcdf4" engine · 8 ✖

author_association 1

id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1446644249 https://github.com/pydata/xarray/pull/6956#issuecomment-1446644249 https://api.github.com/repos/pydata/xarray/issues/6956 IC_kwDOAMm_X85WOgoZ ianliu 30196 2023-02-27T16:27:44Z 2023-02-27T16:27:44Z NONE

@jhamman I think this PR is OK now, what do you think?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Expose `memory` argument for "netcdf4" engine 1352315409
1426909773 https://github.com/pydata/xarray/pull/6956#issuecomment-1426909773 https://api.github.com/repos/pydata/xarray/issues/6956 IC_kwDOAMm_X85VDOpN ianliu 30196 2023-02-12T00:43:09Z 2023-02-12T00:43:09Z NONE

@jhamman sorry for the delay. So, the return that you commented "reupping this question", I did respond the first code review but forgot to submit my answer... I will respond here as well, because I find all these code reviews confusing.

The return statement is there because the netCDF4 API returns the byte array upon closing the dataset. See the API docs here: https://unidata.github.io/netcdf4-python/#in-memory-diskless-datasets

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Expose `memory` argument for "netcdf4" engine 1352315409
1410652829 https://github.com/pydata/xarray/pull/6956#issuecomment-1410652829 https://api.github.com/repos/pydata/xarray/issues/6956 IC_kwDOAMm_X85UFNqd ianliu 30196 2023-01-31T16:05:05Z 2023-01-31T16:05:05Z NONE

Hi @jhamman ! I'm willing to finish it, but don't know whats really missing. I guess I'll rebase with main and fix the failing tests.

But I would like a review on the second commit, since it adds more profound changes to the internal API's, such as returning a value from CachingFileManager.close function. This was needed because NetCDF4 returns the bytearray upon close.

Also, the logic on the to_netcdf function is starting to get a little bit convoluted, would be nice to get a review there as well.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Expose `memory` argument for "netcdf4" engine 1352315409
1232067836 https://github.com/pydata/xarray/pull/6956#issuecomment-1232067836 https://api.github.com/repos/pydata/xarray/issues/6956 IC_kwDOAMm_X85Jb9z8 ianliu 30196 2022-08-30T19:19:46Z 2022-08-30T19:19:46Z NONE

I've just committed a change that also allows one to save a netcdf file to memory, like so:

python ds = xr.Dataset({ "v": xr.DataArray(data=range(10)) }) buf = ds.to_netcdf(engine="netcdf4") assert xr.open_dataset("", engine="netcdf4", memory=buf).equals(ds)

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Expose `memory` argument for "netcdf4" engine 1352315409
1228818566 https://github.com/pydata/xarray/pull/6956#issuecomment-1228818566 https://api.github.com/repos/pydata/xarray/issues/6956 IC_kwDOAMm_X85JPkiG ianliu 30196 2022-08-26T18:58:24Z 2022-08-26T18:58:24Z NONE

@dcherian can you review my changes? I think the PR is done now.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Expose `memory` argument for "netcdf4" engine 1352315409
1228743169 https://github.com/pydata/xarray/pull/6956#issuecomment-1228743169 https://api.github.com/repos/pydata/xarray/issues/6956 IC_kwDOAMm_X85JPSIB ianliu 30196 2022-08-26T17:23:14Z 2022-08-26T17:23:14Z NONE

Hmm, the test doesn't pass like this, it is complaining that nc4 is not defined. I will put the test inside the class.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Expose `memory` argument for "netcdf4" engine 1352315409
1228733153 https://github.com/pydata/xarray/pull/6956#issuecomment-1228733153 https://api.github.com/repos/pydata/xarray/issues/6956 IC_kwDOAMm_X85JPPrh ianliu 30196 2022-08-26T17:11:05Z 2022-08-26T17:11:05Z NONE

@dcherian I've added a unit test, but outside the class. Is this a problem? I see in the Xarray's contributing page that Xarray is transitioning to more functional tests.

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Expose `memory` argument for "netcdf4" engine 1352315409
1228665703 https://github.com/pydata/xarray/pull/6956#issuecomment-1228665703 https://api.github.com/repos/pydata/xarray/issues/6956 IC_kwDOAMm_X85JO_Nn ianliu 30196 2022-08-26T15:54:43Z 2022-08-26T15:55:25Z NONE

@dcherian backend_kwargs doesn't work either, because the NetCDF4BackendEntrypoint.open_dataset function doesn't accept a memory argument:

https://github.com/pydata/xarray/blob/434f9e8929942afc2380eab52a07e77d30cc7885/xarray/backends/netCDF4_.py#L534-L552

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Expose `memory` argument for "netcdf4" engine 1352315409

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