issue_comments

Table actions

8 rows where issue = 1563270549 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

author_association 2

issue 1

  • Update contains_cftime_datetimes to avoid loading entire variable array · 8 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1458453929 https://github.com/pydata/xarray/pull/7494#issuecomment-1458453929 https://api.github.com/repos/pydata/xarray/issues/7494 IC_kwDOAMm_X85W7j2p agoodm 5179430 2023-03-07T16:22:21Z 2023-03-07T16:22:21Z CONTRIBUTOR

Thanks @Illviljan and @dcherian for helping to see this through.

 
{
    "total_count": 2,
    "+1": 2,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Update contains_cftime_datetimes to avoid loading entire variable array 1563270549
1456506306 https://github.com/pydata/xarray/pull/7494#issuecomment-1456506306 https://api.github.com/repos/pydata/xarray/issues/7494 IC_kwDOAMm_X85W0IXC dcherian 2448579 2023-03-06T16:52:13Z 2023-03-06T16:52:13Z MEMBER

Thanks @agoodm this work prompted a bunch of internal cleanup!

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Update contains_cftime_datetimes to avoid loading entire variable array 1563270549
1450003953 https://github.com/pydata/xarray/pull/7494#issuecomment-1450003953 https://api.github.com/repos/pydata/xarray/issues/7494 IC_kwDOAMm_X85WbU3x mathause 10194086 2023-03-01T11:55:41Z 2023-03-01T11:55:41Z MEMBER 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Update contains_cftime_datetimes to avoid loading entire variable array 1563270549
1449323880 https://github.com/pydata/xarray/pull/7494#issuecomment-1449323880 https://api.github.com/repos/pydata/xarray/issues/7494 IC_kwDOAMm_X85WYu1o dcherian 2448579 2023-03-01T04:30:54Z 2023-03-01T04:30:54Z MEMBER

Seems like we're passing a DataArray instead of a Variable somewhere.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Update contains_cftime_datetimes to avoid loading entire variable array 1563270549
1411206291 https://github.com/pydata/xarray/pull/7494#issuecomment-1411206291 https://api.github.com/repos/pydata/xarray/issues/7494 IC_kwDOAMm_X85UHUyT agoodm 5179430 2023-01-31T23:17:38Z 2023-01-31T23:17:38Z CONTRIBUTOR

@Illviljan I gave your update a quick test, it seems to work well enough and still maintains the performance improvement. It looks fine to me though I guess it looks like you still need to fix this failing mypy stuff now?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Update contains_cftime_datetimes to avoid loading entire variable array 1563270549
1411177667 https://github.com/pydata/xarray/pull/7494#issuecomment-1411177667 https://api.github.com/repos/pydata/xarray/issues/7494 IC_kwDOAMm_X85UHNzD Illviljan 14371165 2023-01-31T22:49:24Z 2023-01-31T22:49:24Z MEMBER

@agoodm, what you think of this version? Using xr.Variable directly seems a little easier to work with than trying to guess which type of array (cupy, dask, pint, backendarray, etc) is in the variable.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Update contains_cftime_datetimes to avoid loading entire variable array 1563270549
1410253782 https://github.com/pydata/xarray/pull/7494#issuecomment-1410253782 https://api.github.com/repos/pydata/xarray/issues/7494 IC_kwDOAMm_X85UDsPW agoodm 5179430 2023-01-31T12:22:02Z 2023-01-31T12:26:37Z CONTRIBUTOR

Thanks for the PR. However, does that actually make a difference? To me it looks like _contains_cftime_datetimes also only considers one element of the array.

https://github.com/pydata/xarray/blob/b4515582ffc8b7f63632bfccd109d19889d00384/xarray/core/common.py#L1779-L1780

This isn't actually the line of code that's causing the performance bottleneck, it's the access to var.data in the function call that is actually problematic as I explained in the issue thread. You can verify this yourself running this simple example before and after applying the changes in this PR:

```python import numpy as np import xarray as xr

str_array = np.arange(100000000).astype(str) ds = xr.DataArray(dims=('x',), data=str_array).to_dataset(name='str_array') ds = ds.chunk(x=10000) ds['str_array'] = ds.str_array.astype('O') # Needs to actually be object dtype to show the problem ds.to_zarr('str_array.zarr')

%time xr.open_zarr('str_array.zarr') ```

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Update contains_cftime_datetimes to avoid loading entire variable array 1563270549
1410233955 https://github.com/pydata/xarray/pull/7494#issuecomment-1410233955 https://api.github.com/repos/pydata/xarray/issues/7494 IC_kwDOAMm_X85UDnZj mathause 10194086 2023-01-31T12:06:00Z 2023-01-31T12:06:00Z MEMBER

Thanks for the PR. However, does that actually make a difference? To me it looks like _contains_cftime_datetimes also only considers one element of the array.

https://github.com/pydata/xarray/blob/b4515582ffc8b7f63632bfccd109d19889d00384/xarray/core/common.py#L1779-L1780

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Update contains_cftime_datetimes to avoid loading entire variable array 1563270549

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