issue_comments

Table actions

7 rows where issue = 271017521 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

user 3

author_association 2

issue 1

  • Remove netCDF dependency from rasterio backend tests · 7 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
341994764 https://github.com/pydata/xarray/pull/1687#issuecomment-341994764 https://api.github.com/repos/pydata/xarray/issues/1687 MDEyOklzc3VlQ29tbWVudDM0MTk5NDc2NA== fmaussion 10050469 2017-11-05T18:38:15Z 2017-11-05T18:38:15Z MEMBER

Looks good, thanks! Waiting for the tests to pass 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove netCDF dependency from rasterio backend tests 271017521
341746579 https://github.com/pydata/xarray/pull/1687#issuecomment-341746579 https://api.github.com/repos/pydata/xarray/issues/1687 MDEyOklzc3VlQ29tbWVudDM0MTc0NjU3OQ== fmaussion 10050469 2017-11-03T15:59:30Z 2017-11-03T15:59:30Z MEMBER

@fmaussion Do you have an example of a rasterio object that would fail serialization?

No I don't, but it's more a test for the backend than for the rasterio files. See e.g. https://github.com/pydata/xarray/blob/master/xarray/backends/rasterio_.py#L154 : this line was added by @gbrener and the roundtrip tests obviously forced him to cast the bool to an int . I don't know if its good or bad ;)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove netCDF dependency from rasterio backend tests 271017521
341743560 https://github.com/pydata/xarray/pull/1687#issuecomment-341743560 https://api.github.com/repos/pydata/xarray/issues/1687 MDEyOklzc3VlQ29tbWVudDM0MTc0MzU2MA== maaleske 4414299 2017-11-03T15:49:48Z 2017-11-03T15:49:48Z CONTRIBUTOR

@fmaussion Do you have an example of a rasterio object that would fail serialization?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove netCDF dependency from rasterio backend tests 271017521
341742401 https://github.com/pydata/xarray/pull/1687#issuecomment-341742401 https://api.github.com/repos/pydata/xarray/issues/1687 MDEyOklzc3VlQ29tbWVudDM0MTc0MjQwMQ== maaleske 4414299 2017-11-03T15:46:10Z 2017-11-03T15:46:10Z CONTRIBUTOR

@fmaussion Ah, didn't think of that. That should probably be a separate test though. You're right about the name change, I thought of it already but forgot to change them.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove netCDF dependency from rasterio backend tests 271017521
341742157 https://github.com/pydata/xarray/pull/1687#issuecomment-341742157 https://api.github.com/repos/pydata/xarray/issues/1687 MDEyOklzc3VlQ29tbWVudDM0MTc0MjE1Nw== shoyer 1217238 2017-11-03T15:45:25Z 2017-11-03T15:45:25Z MEMBER

My reasoning (I guess) was that we shouldn't create Dataset objects which cannot be serialized to NetCDF (for whatever reason, for example because we store rasterio objects of some kind), since NetCDF is the preferred way to serialize xarray objects.

I agree that this is a good sanity check.

@maaleske Can we maybe move the netCDF serialization to separate tests that only do that? With the appropriate decorators, of course.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove netCDF dependency from rasterio backend tests 271017521
341740088 https://github.com/pydata/xarray/pull/1687#issuecomment-341740088 https://api.github.com/repos/pydata/xarray/issues/1687 MDEyOklzc3VlQ29tbWVudDM0MTc0MDA4OA== fmaussion 10050469 2017-11-03T15:38:52Z 2017-11-03T15:38:52Z MEMBER

My reasoning (I guess) was that we shouldn't create Dataset objects which cannot be serialized to NetCDF (for whatever reason, for example because we store rasterio objects of some kind), since NetCDF is the preferred way to serialize xarray objects.

I don't mind removing these, but then the test names should be renamed too (test_serialization_platecarree -> test_platecarree)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove netCDF dependency from rasterio backend tests 271017521
341737298 https://github.com/pydata/xarray/pull/1687#issuecomment-341737298 https://api.github.com/repos/pydata/xarray/issues/1687 MDEyOklzc3VlQ29tbWVudDM0MTczNzI5OA== shoyer 1217238 2017-11-03T15:30:00Z 2017-11-03T15:30:00Z MEMBER

I'm not quite sure why @fmaussion added these checks (so I will let him chime in here). I certainly agree that any tests that require writing netCDFs should have the appropriate import checks.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Remove netCDF dependency from rasterio backend tests 271017521

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