home / github

Menu
  • Search all tables
  • GraphQL API

issue_comments

Table actions
  • GraphQL API for issue_comments

9 rows where issue = 467856527 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
  • gwgundersen 3
  • codecov[bot] 1
  • pep8speaks 1

author_association 3

  • MEMBER 4
  • CONTRIBUTOR 3
  • NONE 2

issue 1

  • Return immutable view of Pandas indexes · 9 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
512058648 https://github.com/pydata/xarray/pull/3126#issuecomment-512058648 https://api.github.com/repos/pydata/xarray/issues/3126 MDEyOklzc3VlQ29tbWVudDUxMjA1ODY0OA== max-sixty 5635139 2019-07-17T00:56:58Z 2019-07-17T00:56:58Z MEMBER

Do you have an accidental binary file in the commit? https://github.com/pydata/xarray/pull/3126/files#diff-337006a573a2971594e829114137f91b

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Return immutable view of Pandas indexes 467856527
512031393 https://github.com/pydata/xarray/pull/3126#issuecomment-512031393 https://api.github.com/repos/pydata/xarray/issues/3126 MDEyOklzc3VlQ29tbWVudDUxMjAzMTM5Mw== gwgundersen 2818208 2019-07-16T22:50:04Z 2019-07-16T22:50:04Z CONTRIBUTOR

Okay, a couple changes to the original PR: we now perform a shallow copy of the returned index in DataWithCoords.get_index and Indexes.__getitem__. The latter is needed because both Dataset.indexes and DataArray.indexes returns an Indexes object. Finally, I added a bunch of tests for various index accessors.

This seems pretty robust:

```python

import pandas as pd import xarray as xr index = pd.Index(list('abcd'), name='foo') series = pd.Series(range(4), index) array = xr.DataArray.from_series(series) array.get_index('foo').name = 'bar' array.get_index('foo').name 'foo' array['foo'].to_index().name = 'bar' array['foo'].to_index().name 'foo' array.to_index().name = 'bar' array.to_index().name 'foo' array.indexes['foo'].name = 'bar' array.indexes['foo'].name 'foo' ```

Since a Dataset is just a collection of DataArray objects, the behavior propagates nicely, e.g.

```python

dataset = xr.Dataset({'myvar': array}) dataset['myvar'].to_index().name = 'bar' dataset['myvar'].to_index().name 'foo' dataset.indexes['foo'].name = 'bar' dataset.indexes['foo'].name 'foo' ```

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Return immutable view of Pandas indexes 467856527
512027994 https://github.com/pydata/xarray/pull/3126#issuecomment-512027994 https://api.github.com/repos/pydata/xarray/issues/3126 MDEyOklzc3VlQ29tbWVudDUxMjAyNzk5NA== max-sixty 5635139 2019-07-16T22:36:51Z 2019-07-16T22:36:51Z MEMBER

Do we want all views of indexes to be immutable or do we trust the user to not do stuff like this:

Is there a reasonable way of making that immutable? Given that the index allows its name to be mutated, I'm not sure.

(we can also separate that issue from this change, which is already 👍 )

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Return immutable view of Pandas indexes 467856527
511214715 https://github.com/pydata/xarray/pull/3126#issuecomment-511214715 https://api.github.com/repos/pydata/xarray/issues/3126 MDEyOklzc3VlQ29tbWVudDUxMTIxNDcxNQ== pep8speaks 24736507 2019-07-14T16:02:51Z 2019-07-16T22:33:30Z NONE

Hello @gwgundersen! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2019-07-16 22:33:30 UTC
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Return immutable view of Pandas indexes 467856527
511216252 https://github.com/pydata/xarray/pull/3126#issuecomment-511216252 https://api.github.com/repos/pydata/xarray/issues/3126 MDEyOklzc3VlQ29tbWVudDUxMTIxNjI1Mg== codecov[bot] 22429695 2019-07-14T16:23:30Z 2019-07-16T22:33:26Z NONE

Codecov Report

Merging #3126 into master will decrease coverage by 0.91%. The diff coverage is 100%.

```diff @@ Coverage Diff @@

master #3126 +/-

========================================== - Coverage 95.99% 95.08% -0.92%
========================================== Files 63 63
Lines 12799 12801 +2
========================================== - Hits 12287 12172 -115
- Misses 512 629 +117 ```

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Return immutable view of Pandas indexes 467856527
511716413 https://github.com/pydata/xarray/pull/3126#issuecomment-511716413 https://api.github.com/repos/pydata/xarray/issues/3126 MDEyOklzc3VlQ29tbWVudDUxMTcxNjQxMw== gwgundersen 2818208 2019-07-16T08:20:34Z 2019-07-16T08:21:07Z CONTRIBUTOR

Locally, I've moved my changes from DataArray.to_series to DataWithCoords.get_index, and my tests still pass. After work, I'll write some more tests, e.g. for for DataArray.to_index, and push.

@shoyer, it seems like this issue happens anywhere the user can modify a Pandas index. For example, the Dataset bug in my comment yesterday. Do we want all views of indexes to be immutable or do we trust the user to not do stuff like this:

```python

dataset.indexes['mutable?'].name = 'yes' dataset.indexes['mutable?'].name 'yes' ```

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Return immutable view of Pandas indexes 467856527
511523854 https://github.com/pydata/xarray/pull/3126#issuecomment-511523854 https://api.github.com/repos/pydata/xarray/issues/3126 MDEyOklzc3VlQ29tbWVudDUxMTUyMzg1NA== max-sixty 5635139 2019-07-15T18:48:26Z 2019-07-15T18:48:26Z MEMBER

It looks like to_series calls to_index, which calls get_index,

Thanks for finding that. That sounds right - if we make these changes there then I think we've solved the problem at its root and we should fix all these cases?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Return immutable view of Pandas indexes 467856527
511506569 https://github.com/pydata/xarray/pull/3126#issuecomment-511506569 https://api.github.com/repos/pydata/xarray/issues/3126 MDEyOklzc3VlQ29tbWVudDUxMTUwNjU2OQ== gwgundersen 2818208 2019-07-15T17:59:23Z 2019-07-15T17:59:23Z CONTRIBUTOR

@max-sixty, name is just a mutable property on a Pandas index:

```python

dates = pd.date_range('01-Jan-2019', '31-Jan-2019', name='mutable?') series = pd.Series(np.random.randn(dates.size), dates) series.index.name 'mutable?' series.index.name = 'yes' series.index.name 'yes' ```

But you're right that to_series is the wrong place. It looks like to_series calls to_index, which calls get_index, which is inherited by other classes such as Dataset. For example, the bug persists with Dataset despite my fix:

```python

dates = pd.date_range('01-Jan-2019', '31-Jan-2019', name='mutable?') series = pd.Series(np.random.randn(dates.size), dates) dataset = xr.Dataset({'foo': series}) dataset.indexes['mutable?'].name = 'yes' dataset.indexes['mutable?'].name 'yes' ```

Does anyone know if get_index is the best place for this? Maybe the tests should be somewhere more generic as well.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Return immutable view of Pandas indexes 467856527
511451574 https://github.com/pydata/xarray/pull/3126#issuecomment-511451574 https://api.github.com/repos/pydata/xarray/issues/3126 MDEyOklzc3VlQ29tbWVudDUxMTQ1MTU3NA== max-sixty 5635139 2019-07-15T15:32:09Z 2019-07-15T15:32:09Z MEMBER

I don't quite understand why the previous constructor was returning a mutable view, but the tests look great.

@gwgundersen do you know if we should instead be changing the to_index method which the to_series calls?

Any other comments from anyone?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Return immutable view of Pandas indexes 467856527

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