Repository: crsmithdev/arrow
Pull request #682 (2019-10-01 18:55:50)

Refactored timestamp methods and optimized is_timestamp
The previous implementation of `is_timestamp` was a bit clunky and inefficient, so I refactored it to improve efficiency. I also changed `Arrow.fromtimestamp` and `Arrow.utcfromtimestamp` to use this revamped `is_timestamp` function.

This is in preparation from my next PR which will implement the "x" timestamp token.

codecov-io: # [Codecov](https://codecov.io/gh/crsmithdev/arrow/pull/682?src=pr&el=h1) Report
> Merging [#682](https://codecov.io/gh/crsmithdev/arrow/pull/682?src=pr&el=desc) into [master](https://codecov.io/gh/crsmithdev/arrow/commit/18d346ede24de2b737907db078380e08ceed3bf9?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/crsmithdev/arrow/pull/682/graphs/tree.svg?width=650&token=4EkGnFsIGn&height=150&src=pr)](https://codecov.io/gh/crsmithdev/arrow/pull/682?src=pr&el=tree)

```diff
@@          Coverage Diff          @@
##           master   #682   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           8      8           
  Lines        1549   1547    -2     
  Branches      247    250    +3     
=====================================
- Hits         1549   1547    -2
```


| [Impacted Files](https://codecov.io/gh/crsmithdev/arrow/pull/682?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [arrow/factory.py](https://codecov.io/gh/crsmithdev/arrow/pull/682/diff?src=pr&el=tree#diff-YXJyb3cvZmFjdG9yeS5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [arrow/util.py](https://codecov.io/gh/crsmithdev/arrow/pull/682/diff?src=pr&el=tree#diff-YXJyb3cvdXRpbC5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [arrow/arrow.py](https://codecov.io/gh/crsmithdev/arrow/pull/682/diff?src=pr&el=tree#diff-YXJyb3cvYXJyb3cucHk=) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/crsmithdev/arrow/pull/682?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/crsmithdev/arrow/pull/682?src=pr&el=footer). Last update [18d346e...00abd8f](https://codecov.io/gh/crsmithdev/arrow/pull/682?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/arrow/arrow.py b/arrow/arrow.py
index 1b05b42..3790b35 100644
--- a/arrow/arrow.py
+++ b/arrow/arrow.py
@@ -150,8 +150,13 @@ class Arrow(object):
 
         if tzinfo is None:
             tzinfo = dateutil_tz.tzlocal()
-        timestamp = cls._get_timestamp_from_input(timestamp)
-        dt = datetime.fromtimestamp(timestamp, tzinfo)
+
+        if not util.is_timestamp(timestamp):
+            raise ValueError(
+                "The provided timestamp '{}' is invalid.".format(timestamp)
+            )
+
+        dt = datetime.fromtimestamp(float(timestamp), tzinfo)
 
         return cls(
             dt.year,
@@ -172,8 +177,12 @@ class Arrow(object):
 
         """
 
-        timestamp = cls._get_timestamp_from_input(timestamp)
-        dt = datetime.utcfromtimestamp(timestamp)
+        if not util.is_timestamp(timestamp):
+            raise ValueError(
+                "The provided timestamp '{}' is invalid.".format(timestamp)
+            )
+
+        dt = datetime.utcfromtimestamp(float(timestamp))
 
         return cls(
             dt.year,
@@ -1382,14 +1391,6 @@ class Arrow(object):
                 return end, sys.maxsize
             return end, limit
 
-    @staticmethod
-    def _get_timestamp_from_input(timestamp):
-
-        try:
-            return float(timestamp)
-        except Exception:
-            raise ValueError("cannot parse '{}' as a timestamp".format(timestamp))
-
 
 Arrow.min = Arrow.fromdatetime(datetime.min)
 Arrow.max = Arrow.fromdatetime(datetime.max)
diff --git a/arrow/factory.py b/arrow/factory.py
index f9f2d65..8bdccc9 100644
--- a/arrow/factory.py
+++ b/arrow/factory.py
@@ -169,19 +169,19 @@ class ArrowFactory(object):
                 return self.type.utcnow()
 
             # try (int, float) -> utc, from timestamp.
-            if is_timestamp(arg):
+            elif not isstr(arg) and is_timestamp(arg):
                 return self.type.utcfromtimestamp(arg)
 
             # (Arrow) -> from the object's datetime.
-            if isinstance(arg, Arrow):
+            elif isinstance(arg, Arrow):
                 return self.type.fromdatetime(arg.datetime)
 
             # (datetime) -> from datetime.
-            if isinstance(arg, datetime):
+            elif isinstance(arg, datetime):
                 return self.type.fromdatetime(arg)
 
             # (date) -> from date.
-            if isinstance(arg, date):
+            elif isinstance(arg, date):
                 return self.type.fromdate(arg)
 
             # (tzinfo) -> now, @ tzinfo.
@@ -204,7 +204,7 @@ class ArrowFactory(object):
 
             else:
                 raise TypeError(
-                    "Can't parse single argument type of '{}'".format(type(arg))
+                    "Can't parse single argument of type '{}'".format(type(arg))
                 )
 
         elif arg_count == 2:
@@ -242,7 +242,7 @@ class ArrowFactory(object):
 
             else:
                 raise TypeError(
-                    "Can't parse two arguments of types '{}', '{}'".format(
+                    "Can't parse two arguments of types '{}' and '{}'".format(
                         type(arg_1), type(arg_2)
                     )
                 )
diff --git a/arrow/util.py b/arrow/util.py
index 7faea5c..62f1a05 100644
--- a/arrow/util.py
+++ b/arrow/util.py
@@ -9,12 +9,17 @@ def total_seconds(td):  # pragma: no cover
 
 
 def is_timestamp(value):
+    """Check if value is a valid timestamp."""
     if isinstance(value, bool):
         return False
+    if not (
+        isinstance(value, int) or isinstance(value, float) or isinstance(value, str)
+    ):
+        return False
     try:
-        datetime.datetime.fromtimestamp(value)
+        float(value)
         return True
-    except TypeError:
+    except ValueError:
         return False
 
 


Test patch:
diff --git a/tests/arrow_tests.py b/tests/arrow_tests.py
index f243655..4b26e61 100644
--- a/tests/arrow_tests.py
+++ b/tests/arrow_tests.py
@@ -103,6 +103,19 @@ class ArrowFactoryTests(Chai):
             datetime.fromtimestamp(timestamp, tz.gettz("Europe/Paris")),
         )
 
+        with self.assertRaises(ValueError):
+            arrow.Arrow.fromtimestamp("invalid timestamp")
+
+    def test_utcfromtimestamp(self):
+
+        timestamp = time.time()
+
+        result = arrow.Arrow.utcfromtimestamp(timestamp)
+        assertDtEqual(result._datetime, datetime.utcnow().replace(tzinfo=tz.tzutc()))
+
+        with self.assertRaises(ValueError):
+            arrow.Arrow.utcfromtimestamp("invalid timestamp")
+
     def test_fromdatetime(self):
 
         dt = datetime(2013, 2, 3, 12, 30, 45, 1)
@@ -1805,16 +1818,6 @@ class ArrowUtilTests(Chai):
             get_tzinfo("abc")
         self.assertFalse("{}" in str(raise_ctx.exception))
 
-    def test_get_timestamp_from_input(self):
-
-        self.assertEqual(arrow.Arrow._get_timestamp_from_input(123), 123)
-        self.assertEqual(arrow.Arrow._get_timestamp_from_input(123.4), 123.4)
-        self.assertEqual(arrow.Arrow._get_timestamp_from_input("123"), 123.0)
-        self.assertEqual(arrow.Arrow._get_timestamp_from_input("123.4"), 123.4)
-
-        with self.assertRaises(ValueError):
-            arrow.Arrow._get_timestamp_from_input("abc")
-
     def test_get_iteration_params(self):
 
         self.assertEqual(
diff --git a/tests/factory_tests.py b/tests/factory_tests.py
index 3b4c7a9..98d9eca 100644
--- a/tests/factory_tests.py
+++ b/tests/factory_tests.py
@@ -68,8 +68,8 @@ class GetTests(Chai):
             self.factory.get(str(float_timestamp))
 
         # Regression test for issue #216
-        timestamp = 99999999999999999999999999
-        # Python 3 raises `OverflowError`, Python 2 raises `ValueError`
+        # Python 3 raises OverflowError, Python 2 raises ValueError
+        timestamp = 99999999999999999999999999.99999999999999999999999999
         with self.assertRaises((OverflowError, ValueError)):
             self.factory.get(timestamp)
 
diff --git a/tests/util_tests.py b/tests/util_tests.py
index 21e98c7..bb84c44 100644
--- a/tests/util_tests.py
+++ b/tests/util_tests.py
@@ -13,22 +13,19 @@ class UtilTests(Chai):
 
         self.assertTrue(util.is_timestamp(timestamp_int))
         self.assertTrue(util.is_timestamp(timestamp_float))
+        self.assertTrue(util.is_timestamp(str(timestamp_int)))
+        self.assertTrue(util.is_timestamp(str(timestamp_float)))
 
-        self.assertFalse(util.is_timestamp(str(timestamp_int)))
-        self.assertFalse(util.is_timestamp(str(timestamp_float)))
         self.assertFalse(util.is_timestamp(True))
         self.assertFalse(util.is_timestamp(False))
 
-        full_datetime = "2019-06-23T13:12:42"
-        self.assertFalse(util.is_timestamp(full_datetime))
+        class InvalidTimestamp:
+            pass
 
-        overflow_timestamp_float = 99999999999999999999999999.99999999999999999999999999
-        with self.assertRaises((OverflowError, ValueError)):
-            util.is_timestamp(overflow_timestamp_float)
+        self.assertFalse(util.is_timestamp(InvalidTimestamp()))
 
-        overflow_timestamp_int = int(overflow_timestamp_float)
-        with self.assertRaises((OverflowError, ValueError)):
-            util.is_timestamp(overflow_timestamp_int)
+        full_datetime = "2019-06-23T13:12:42"
+        self.assertFalse(util.is_timestamp(full_datetime))
 
     def test_iso_gregorian(self):
         with self.assertRaises(ValueError):