issue_comments

Table actions

7 rows where issue = 1471685307 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 4

issue 1

  • Disable bottleneck by default? · 7 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1562040566 https://github.com/pydata/xarray/issues/7344#issuecomment-1562040566 https://api.github.com/repos/pydata/xarray/issues/7344 IC_kwDOAMm_X85dGtj2 riley-brady 82663402 2023-05-24T23:12:48Z 2023-05-24T23:12:48Z NONE

I want to add a +1 to disable it by default. It's pretty common to be using float32 precision arrays. I have a rolling mean operation early on in some code and the errors balloon over time in subsequent processes. This was a super obscure bug to track down as well.

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Disable bottleneck by default? 1471685307
1464384781 https://github.com/pydata/xarray/issues/7344#issuecomment-1464384781 https://api.github.com/repos/pydata/xarray/issues/7344 IC_kwDOAMm_X85XSL0N headtr1ck 43316012 2023-03-10T20:31:28Z 2023-03-10T20:31:28Z COLLABORATOR

Maybe it is just a problem of documenting it more clearly?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Disable bottleneck by default? 1471685307
1351908915 https://github.com/pydata/xarray/issues/7344#issuecomment-1351908915 https://api.github.com/repos/pydata/xarray/issues/7344 IC_kwDOAMm_X85QlH4z shoyer 1217238 2022-12-14T18:24:04Z 2022-12-14T18:24:04Z MEMBER

I think it's OK to still require bottleneck for ffill and bfill:

  1. There are no numerical concerns: these functions simply repeat numbers forward (or backwards).
  2. There is no good alternative to using a loop, and writing the loop in NumPy would be probitively slow.
 
{
    "total_count": 2,
    "+1": 2,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Disable bottleneck by default? 1471685307
1351398850 https://github.com/pydata/xarray/issues/7344#issuecomment-1351398850 https://api.github.com/repos/pydata/xarray/issues/7344 IC_kwDOAMm_X85QjLXC markelg 6883049 2022-12-14T13:54:06Z 2022-12-14T13:54:06Z CONTRIBUTOR

I fully agree that correctness is the priority. Note however that some functions now require bottleneck, like ffill and bfill (I am not sure if there are more cases). They may need to be modified so they can run without bottleneck.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Disable bottleneck by default? 1471685307
1336299057 https://github.com/pydata/xarray/issues/7344#issuecomment-1336299057 https://api.github.com/repos/pydata/xarray/issues/7344 IC_kwDOAMm_X85Ppk4x shoyer 1217238 2022-12-04T01:55:34Z 2022-12-04T01:55:34Z MEMBER

The case where Bottleneck really makes a difference was for moving window statistics, where it uses a smarter algorithm than our current NumPy implementation, which creating a moving window view.

Otherwise, I agree, it probably isn't worth the trouble.

That said -- we could also switch to smarter NumPy based algorithms to implement most moving window calculations, e.g,. using np.nancumsum for moving window means.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Disable bottleneck by default? 1471685307
1334523276 https://github.com/pydata/xarray/issues/7344#issuecomment-1334523276 https://api.github.com/repos/pydata/xarray/issues/7344 IC_kwDOAMm_X85PizWM max-sixty 5635139 2022-12-01T22:22:02Z 2022-12-01T22:22:02Z MEMBER

I'd be fine with disabling, since bottleneck doesn't seem to be actively maintained.

Though I would say it's numerically unstable rather than incorrect! I would always want it enabled, but it does make sense to default to the conservative option.

I had dreams of making numbagg into a better bottleneck — it's just as fast, much more flexible, and integrates really well with xarray. But those dreams have not come to pass (yet!).

 
{
    "total_count": 2,
    "+1": 2,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Disable bottleneck by default? 1471685307
1334242366 https://github.com/pydata/xarray/issues/7344#issuecomment-1334242366 https://api.github.com/repos/pydata/xarray/issues/7344 IC_kwDOAMm_X85Phuw- TomNicholas 35968931 2022-12-01T19:24:24Z 2022-12-01T19:24:24Z MEMBER

I kinda think correctness by default is more important than performance, especially if the default performance isn't prohibitively slow. 

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Disable bottleneck by default? 1471685307

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