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 2022/11/03 15:17:25 UTC

[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #13718: ARROW-15026: [Python] Error if datetime.timedelta to pyarrow.duration conversion overflows

jorisvandenbossche commented on code in PR #13718:
URL: https://github.com/apache/arrow/pull/13718#discussion_r1013031700


##########
python/pyarrow/src/arrow/python/datetime.h:
##########
@@ -20,9 +20,13 @@
 #include <algorithm>
 #include <chrono>
 
+#include "arrow/python/platform.h"
+#include "arrow/python/visibility.h"

Review Comment:
   I suppose this is a left-over from rebasing on top of the moved code (those lines can be removed, they are already included a bit below as well)



##########
python/pyarrow/tests/test_types.py:
##########
@@ -857,6 +857,31 @@ def test_decimal_overflow():
             pa.decimal256(i, 0)
 
 
+def test_timedelta_overflow():
+    # microsecond resolution, overflow
+    d = datetime.timedelta(days=-106751992, seconds=71945, microseconds=224192)
+    with pytest.raises(pa.ArrowInvalid):
+        pa.scalar(d)
+
+    # microsecond resolution, overflow
+    d = datetime.timedelta(days=106751992, seconds=14454, microseconds=775807)

Review Comment:
   This one will overflow based on the days alone. We can also add a case that will only overflow when adding the microseconds. I think `datetime.timedelta(days=106751991, seconds=14454, microseconds=775808)` should to the trick (`datetime.timedelta(days=106751991, seconds=14454, microseconds=775807)` is the limit that works, so adding one more to the microseconds)



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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