issues

Table actions

2 rows where type = "pull" and user = 42680748 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

state 2

type 1

  • pull · 2 ✖

repo 1

id node_id number title user state locked assignee milestone comments created_at updated_at ▲ closed_at author_association active_lock_reason draft pull_request body reactions performed_via_github_app state_reason repo type
2060883540 PR_kwDOAMm_X85i-ZWI 8577 Interpolate na: Fix #7665 and introduce arguments similar to pandas Ockenfuss 42680748 open 0     0 2023-12-30T23:28:47Z 2023-12-30T23:28:47Z   CONTRIBUTOR   0 pydata/xarray/pulls/8577
  • [x] Closes #7665
  • [x] Tests added
  • [ ] User visible changes (including notable bug fixes) are documented in whats-new.rst

This is an attempt to close #7665 and combine the current possibilities from xarray (max_gap) and pandas (limit_direction, limit_area) regarding interpolation of nan values. Please see also my comments in #7665 for the motivation. This PR already involves a full implementation, documentation and corresponding tests, but before any final polishing, I want to hear your thoughts. Specifically, I think the API and default options need to be discussed. (See the proposed documentation of DataArray.interpolate_na() / Dataset.interpolate_na() for the current state)

Implementation: Basically, I use ffill and bfill to calculate the coordinate of the left/right edge for every gap in the data. Based on edge coordinates, all masks (limit, limit_area, max_gap) are created.

On the long term, it might be interesting to provide those arguments to other na-filling methods as well (ffill, bfill, fillna).

Things to consider

limit_direction=forward

Pros: - Backward compatible: If limit is not None, this is the current behaviour (see #7665) - Pandas compatible: Forward is the pandas default.

Cons: - limit_direction=both feels more natural as default. If the user does interpolate_na('x', fill_value='extrapolate'), in my opinion they will expect all nans to be filled, including both boundaries. In contrast to pandas, this was the case in xarray before, but not anymore now if we follow pandas and set limit_direction=forward. both would also increase performance, since no restrictions need to be applied.

limit_use_coordinates=False

Pros: - Backward compatible - Pandas compatible -> Both xarray and pandas have no support for coordinate based limits so far.

Cons: - Inconsistent with the current default of use_coordinates=True

Generally, one might discuss if this separate argument is necessary or only one argument use_coordinates is sufficient. Imo, if the grid is irregular and use_coordinates=True, there is not a lot of sense in specifying the limit as a fixed number of grid cells. Alternatively, we could allow a three-tuple like use_coordinates=(True, True, False) to specify the index for interpolation, limit and max_gap separately (or something similar).

use_coordinates=True

So far, if there is no coordinate for dim, interpolation will succeed, falling silently back to a linearly increasing index. I feel, for use_coordinate=True, we should fail and inform the user to set use_coordinate=False if they really want a linear index. However, this is a breaking change. Maybe we can keep this behaviour with use_coordinate=None as new default option (= True if coord existent, else linear).

Performance

On my machine, the new limit implementation based on ffill/bfill seems to be a little less performant (10%) than the old one (based on rolling). There might be potential for improvements.

 
{
    "url": "https://api.github.com/repos/pydata/xarray/issues/8577/reactions",
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
     xarray 13221727 pull
1615645463 PR_kwDOAMm_X85Ll0US 7598 Fix missing 'dim' argument in _get_nan_block_lengths Ockenfuss 42680748 closed 0     3 2023-03-08T17:28:56Z 2023-03-23T16:04:32Z 2023-03-16T18:55:56Z CONTRIBUTOR   0 pydata/xarray/pulls/7598
  • Add missing dim argument (GH7597)

  • Append a nan gap at the end of existing tests cases

  • [x] Closes #7597

  • [x] Tests added
  • [ ] User visible changes (including notable bug fixes) are documented in whats-new.rst
 
{
    "url": "https://api.github.com/repos/pydata/xarray/issues/7598/reactions",
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
     xarray 13221727 pull

Advanced export

JSON shape: default, array, newline-delimited, object

CSV options: 

CREATE TABLE [issues] (
   [id] INTEGER PRIMARY KEY,
   [node_id] TEXT,
   [number] INTEGER,
   [title] TEXT,
   [user] INTEGER REFERENCES [users]([id]),
   [state] TEXT,
   [locked] INTEGER,
   [assignee] INTEGER REFERENCES [users]([id]),
   [milestone] INTEGER REFERENCES [milestones]([id]),
   [comments] INTEGER,
   [created_at] TEXT,
   [updated_at] TEXT,
   [closed_at] TEXT,
   [author_association] TEXT,
   [active_lock_reason] TEXT,
   [draft] INTEGER,
   [pull_request] TEXT,
   [body] TEXT,
   [reactions] TEXT,
   [performed_via_github_app] TEXT,
   [state_reason] TEXT,
   [repo] INTEGER REFERENCES [repos]([id]),
   [type] TEXT
);
CREATE INDEX [idx_issues_repo]
    ON [issues] ([repo]);
CREATE INDEX [idx_issues_milestone]
    ON [issues] ([milestone]);
CREATE INDEX [idx_issues_assignee]
    ON [issues] ([assignee]);
CREATE INDEX [idx_issues_user]
    ON [issues] ([user]);