issue_comments

Table actions

8 rows where issue = 1389176083 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

author_association 3

issue 1

  • Don't cast NaN to integer · 8 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
1510407267 https://github.com/pydata/xarray/pull/7098#issuecomment-1510407267 https://api.github.com/repos/pydata/xarray/issues/7098 IC_kwDOAMm_X85aBvxj kxxt 18085551 2023-04-16T15:01:24Z 2023-04-16T15:01:24Z NONE

Hi, is there any reason that keeps this pr from being merged? This PR is a fix for some failing tests on riscv64.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Don't cast NaN to integer 1389176083
1263671832 https://github.com/pydata/xarray/pull/7098#issuecomment-1263671832 https://api.github.com/repos/pydata/xarray/issues/7098 IC_kwDOAMm_X85LUhoY dcherian 2448579 2022-09-30T14:47:06Z 2022-09-30T14:47:06Z MEMBER

I think the real solution here is to explicitly handle NaNs during the decoding step. We do want these to be NaT in the output.

cc @spencerclark

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Don't cast NaN to integer 1389176083
1262836815 https://github.com/pydata/xarray/pull/7098#issuecomment-1262836815 https://api.github.com/repos/pydata/xarray/issues/7098 IC_kwDOAMm_X85LRVxP TomNicholas 35968931 2022-09-29T21:28:31Z 2022-09-29T21:28:31Z MEMBER

Thank you very much for the context @DocOtak !

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Don't cast NaN to integer 1389176083
1262828844 https://github.com/pydata/xarray/pull/7098#issuecomment-1262828844 https://api.github.com/repos/pydata/xarray/issues/7098 IC_kwDOAMm_X85LRT0s DocOtak 868027 2022-09-29T21:22:32Z 2022-09-29T21:22:32Z CONTRIBUTOR

@TomNicholas Something different will need to happen with that cast eventually. See #6191 for something that is failing on some systems that users have but is currently unable to be captured in the tests. Numpy has already added runtime warnings about doing this, and is "thinking about" making nan to int casts raise https://github.com/numpy/numpy/issues/14412. Xarray's own @shoyer has hit issues like this before as well https://github.com/numpy/numpy/issues/6109.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Don't cast NaN to integer 1389176083
1262637254 https://github.com/pydata/xarray/pull/7098#issuecomment-1262637254 https://api.github.com/repos/pydata/xarray/issues/7098 IC_kwDOAMm_X85LQlDG andreas-schwab 2175493 2022-09-29T18:08:29Z 2022-09-29T18:08:29Z NONE

That's the beauty of undefined behaviour. It comes in many flavors, and you never know which one you get. Which also means that it is impossible to reliably test for it.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Don't cast NaN to integer 1389176083
1262579935 https://github.com/pydata/xarray/pull/7098#issuecomment-1262579935 https://api.github.com/repos/pydata/xarray/issues/7098 IC_kwDOAMm_X85LQXDf TomNicholas 35968931 2022-09-29T17:20:48Z 2022-09-29T17:20:48Z MEMBER

It's already perfectly covered by the testsuite.

Okay great, but then why are these tests failing for you (locally?) and not in our CI runs? (Our main branch just passed all automated tests just now.) Do you have a different version of some package that we aren't testing against?

I'm asking because if our current CI test runs don't reproduce this error, then we (a) have no way to check that your change fixed the error and (b) will not know if some regression causes this error to resurface in future. Standard practice would be to raise an issue that demonstrates the problem, then link the fix PR to that issue.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Don't cast NaN to integer 1389176083
1262568339 https://github.com/pydata/xarray/pull/7098#issuecomment-1262568339 https://api.github.com/repos/pydata/xarray/issues/7098 IC_kwDOAMm_X85LQUOT andreas-schwab 2175493 2022-09-29T17:12:43Z 2022-09-29T17:12:43Z NONE

It's already perfectly covered by the testsuite.

=========================== short test summary info ============================ FAILED xarray/tests/test_backends.py::TestScipyInMemoryData::test_roundtrip_numpy_datetime_data FAILED xarray/tests/test_backends.py::TestScipyFileObject::test_roundtrip_numpy_datetime_data FAILED xarray/tests/test_backends.py::TestGenericNetCDFData::test_roundtrip_numpy_datetime_data FAILED xarray/tests/test_backends.py::TestScipyFilePath::test_roundtrip_numpy_datetime_data = 4 failed, 4636 passed, 5632 skipped, 19 xfailed, 22 xpassed, 38 warnings in 266.18s (0:04:26) =

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Don't cast NaN to integer 1389176083
1262537736 https://github.com/pydata/xarray/pull/7098#issuecomment-1262537736 https://api.github.com/repos/pydata/xarray/issues/7098 IC_kwDOAMm_X85LQMwI TomNicholas 35968931 2022-09-29T16:44:38Z 2022-09-29T16:44:38Z MEMBER

Hi @andreas-schwab, and welcome to xarray!

Can you tell what specific failures this change fixed for you? If there was something failing we want to capture it in our test suite, but I am not sure what failure you are referring to.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Don't cast NaN to integer 1389176083

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