You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/05/21 14:42:22 UTC

[GitHub] [arrow] lidavidm opened a new pull request #10375: ARROW-12843: [C++][R] Implement is_inf kernel

lidavidm opened a new pull request #10375:
URL: https://github.com/apache/arrow/pull/10375


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] cyb70289 commented on pull request #10375: ARROW-12843: [C++][R] Implement is_inf kernel

Posted by GitBox <gi...@apache.org>.
cyb70289 commented on pull request #10375:
URL: https://github.com/apache/arrow/pull/10375#issuecomment-849252521


   @ianmcook , do you have other comments? Can we merge this PR?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] cyb70289 closed pull request #10375: ARROW-12843: [C++][R] Implement is_inf kernel

Posted by GitBox <gi...@apache.org>.
cyb70289 closed pull request #10375:
URL: https://github.com/apache/arrow/pull/10375


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] github-actions[bot] commented on pull request #10375: ARROW-12843: [C++][R] Implement is_inf kernel

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10375:
URL: https://github.com/apache/arrow/pull/10375#issuecomment-846020393


   https://issues.apache.org/jira/browse/ARROW-12843


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] lidavidm commented on pull request #10375: ARROW-12843: [C++][R] Implement is_inf kernel

Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #10375:
URL: https://github.com/apache/arrow/pull/10375#issuecomment-846262039


   Oh, doh. Yes, then having an is_finite kernel would make more sense than just inverting it. I'll update that.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] ianmcook commented on pull request #10375: ARROW-12843: [C++][R] Implement is_inf kernel

Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #10375:
URL: https://github.com/apache/arrow/pull/10375#issuecomment-846261638


   The fact that `is_inf` returns false for `NaN` is inconvenient because its negation returns true.
   
   Both R and NumPy have separate functions to test for finite and invite values, and these all return false for `NaN`:
   ```r
   > is.infinite(NaN)
   [1] FALSE
   > is.finite(NaN)
   [1] FALSE
   ```
   ```python
   >>> np.isinf(np.nan)
   False
   >>> np.isfinite(np.nan)
   False
   ```
   I wonder if this suggests we should also make an `is_fin` or `is_finite` kernel to avoid this problem of negation.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] lidavidm commented on pull request #10375: ARROW-12843: [C++][R] Implement is_inf kernel

Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #10375:
URL: https://github.com/apache/arrow/pull/10375#issuecomment-846022488


   Yeah, please go ahead! I admit I'm not familiar with the R side of things and mostly just pattern match on similar code.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] ianmcook commented on pull request #10375: ARROW-12843: [C++][R] Implement is_inf kernel

Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #10375:
URL: https://github.com/apache/arrow/pull/10375#issuecomment-846293975


   This also fixes an inconsistency with the `is.na()` method for `Scalar` objects in the R package. Previously that returned a length-1 R logical vector. Now it returns a `Scalar` having the `bool` data type. This was necessary go make `is.finite()` and `is.infinite()` work consistently.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] ianmcook commented on pull request #10375: ARROW-12843: [C++][R] Implement is_inf kernel

Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #10375:
URL: https://github.com/apache/arrow/pull/10375#issuecomment-846021866


   Thanks for doing this @lidavidm! You mind if I push a commit to your fork to tweak some of the R stuff?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] ianmcook commented on pull request #10375: ARROW-12843: [C++][R] Implement is_inf kernel

Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #10375:
URL: https://github.com/apache/arrow/pull/10375#issuecomment-849260492


   @cyb70289 No more comments from me, looks good, thanks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org