issue_comments

Table actions

2 rows where issue = 485708282 and user = 10050469 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

user 1

  • fmaussion · 2 ✖

issue 1

  • Stateful user-defined accessors · 2 ✖

author_association 1

id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
525268978 https://github.com/pydata/xarray/issues/3268#issuecomment-525268978 https://api.github.com/repos/pydata/xarray/issues/3268 MDEyOklzc3VlQ29tbWVudDUyNTI2ODk3OA== fmaussion 10050469 2019-08-27T12:00:02Z 2019-08-27T12:00:02Z MEMBER

could you change your accessor code to store its state in Dataset.attrs instead?

Yes, although attrs would be bad as well because they are lost in many operations. The current "best practice" would be to use coords for this, as in https://github.com/pydata/xarray/issues/1271#issuecomment-280574680

I never bothered to get this done (my library is quite niche and not mean to become widely used), but if we remove the caching mechanism in xarray, it would be one more incentive to make the switch.

So you work on the assumption that no new potentially useful coords will be added after the first invocation of your accessor?

Yes. In almost all use cases I know of this won't happen. Typically, the users have to create (or open) an xarray object that salem understands before calling the accessor anyway.

Or do you have logic that invalidates your cache every time the state of the coords changes?

No. I didn't think about that before today ;-). But you are right: if users of salem's accessor change the coordinates after calling the accessor, then bad things might happen. Experience shows that this rarely happens (never happened to me), but I can see how this can fire back.

Altogether, these are good arguments to remove the caching mechanism I believe.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Stateful user-defined accessors 485708282
525252528 https://github.com/pydata/xarray/issues/3268#issuecomment-525252528 https://api.github.com/repos/pydata/xarray/issues/3268 MDEyOklzc3VlQ29tbWVudDUyNTI1MjUyOA== fmaussion 10050469 2019-08-27T11:03:47Z 2019-08-27T11:04:10Z MEMBER

Interesting, thanks!

As an accessor maintainer, I can ensure that at least one accessor implementation is storing state ;-). But this state is based on the xarray object itself: for example, we derive georeferencing information and store the names of the coordinate variabless we know are going to be useful to us later. That is, every new call to __init__ based on a modified object will trigger a new parsing, and we don't come into the situation you describe above.

Getting rid of the caching logic would mean some performance loss to us, yes, but I don't know if it's "worse" than the circular reference issue you describe or not.

 
{
    "total_count": 1,
    "+1": 0,
    "-1": 0,
    "laugh": 1,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Stateful user-defined accessors 485708282

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