Skip to content

Complete typing with strict type-checking #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
- name: run the tests
run: python3 tests.py
- name: verify type hints
run: mypy datemath
run: mypy . --strict
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running on tests is important too. This ensures expected usage is correctly typed. In fact, it found that parse was missing the int annotation on its first parameter.

- name: verify package install
run: python3 setup.py install --user
- name: verify we can import
Expand Down
16 changes: 13 additions & 3 deletions datemath/__init__.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
from .helpers import parse, DateMathException
from __future__ import annotations

Copy link
Contributor Author

@Avasam Avasam Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows the use of | in Pyton < 3.10, and use symbols that won't be present at runtime, in annotations.

def dm(expr, **kwargs):
from datetime import datetime
from typing import TYPE_CHECKING

from arrow import Arrow

if TYPE_CHECKING:
from typing_extensions import Unpack

from .helpers import ParseParams, parse as parse, DateMathException as DateMathException

Copy link
Contributor Author

@Avasam Avasam Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as imports without renaming is a typing convention understood by type-checkers to mean that the symbol is purposefully / explicitly re-exported (the other option is to use __all__)

def dm(expr: str | int, **kwargs: Unpack[ParseParams]) -> Arrow:
''' does our datemath and returns an arrow object '''
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By using an unpacked TypedDict to annotate kwargs, you don't have to duplicate all your default keyword argument parameter's default values to have these functions fully typed.

return parse(expr, **kwargs)

def datemath(expr, **kwargs):
def datemath(expr: str | int, **kwargs: Unpack[ParseParams]) -> datetime:
''' does our datemath and returns a datetime object '''
return parse(expr, **kwargs).datetime
39 changes: 22 additions & 17 deletions datemath/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,14 @@

'''

from __future__ import annotations

import os
import re
from typing import TypedDict, cast

import arrow
from arrow import Arrow
from datetime import datetime
import re
import os
from dateutil import tz
import dateutil
import sys
from pprint import pprint
from typing import Any, Optional

debug = True if os.environ.get('DATEMATH_DEBUG') else False

Expand Down Expand Up @@ -78,7 +76,13 @@ def unitMap(c: str) -> str:
else:
raise DateMathException("Not a valid timeunit: {0}".format(c))

def parse(expression: str, now: Any = None, tz: str = 'UTC', type: Any = None, roundDown: bool = True) -> Arrow:
class ParseParams(TypedDict, total=False):
Copy link
Contributor Author

@Avasam Avasam Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment in datemath/__init__.py
total=False means all members are optional (since we use this to annotate optional kwargs)

now: Arrow | None
tz: str
type: str | None
roundDown: bool

def parse(expression: str | int, now: Arrow | None = None, tz: str = 'UTC', type: str | None = None, roundDown: bool = True) -> Arrow:
'''
the main meat and potatoes of this this whole thing
takes our datemath expression and does our date math
Expand All @@ -101,15 +105,16 @@ def parse(expression: str, now: Any = None, tz: str = 'UTC', type: Any = None, r
if debug: print("parse() - will now convert tz to {0}".format(tz))
now = now.to(tz)

expression = str(expression)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Less runtime casting in some cases. But more importantly, this is now type-safe.

if expression == 'now':
if debug: print("parse() - Now, no dm: {0}".format(now))
if type:
return getattr(now, type)
return cast(Arrow, getattr(now, type))
Copy link
Contributor Author

@Avasam Avasam Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In strict mode, mypy will whine about returning Any from a typed function.
This can either be cast or type-ignored to accept the "type unsafety".

Suggested change
return cast(Arrow, getattr(now, type))
return getattr(now, type) # type: ignore[no-any-return]

else:
return now
elif re.match(r'\d{10,}', str(expression)):
elif re.match(r'\d{10,}', expression):
if debug: print('parse() - found an epoch timestamp')
if len(str(expression)) == 13:
if len(expression) == 13:
raise DateMathException('Unable to parse epoch timestamps in millis, please convert to the nearest second to continue - i.e. 1451610061 / 1000')
ts = arrow.get(int(expression))
ts = ts.replace(tzinfo=tz)
Expand Down Expand Up @@ -142,7 +147,7 @@ def parse(expression: str, now: Any = None, tz: str = 'UTC', type: Any = None, r
rettime = evaluate(math, time, tz, roundDown)

if type:
return getattr(rettime, type)
return cast(Arrow, getattr(rettime, type))
Copy link
Contributor Author

@Avasam Avasam Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In strict mode, mypy will whine about returning Any from a typed function.
This can either be cast or type-ignored to accept the "type unsafety".

Suggested change
return cast(Arrow, getattr(rettime, type))
return getattr(rettime, type) # type: ignore[no-any-return]

else:
return rettime

Expand All @@ -158,7 +163,7 @@ def parseTime(timestamp: str, timezone: str = 'UTC') -> Arrow:
if debug: print("parseTime() - timezone that came in = {}".format(timezone))

if ts.tzinfo:
import dateutil
import dateutil.tz
if isinstance(ts.tzinfo, dateutil.tz.tz.tzoffset):
# this means our TZ probably came in via our datetime string
# then lets set our tz to whatever tzoffset is
Expand All @@ -175,14 +180,14 @@ def parseTime(timestamp: str, timezone: str = 'UTC') -> Arrow:
if debug: print('parseTime() - Doesnt look like we have a valid timestamp, raise an exception. timestamp={}'.format(timestamp))
raise DateMathException('Valid length timestamp not provide, you gave me a timestamp of "{}", but I need something that has a len() >= 4'.format(timestamp))

def roundDate(now: Any, unit: str, tz: str = 'UTC', roundDown: bool = True) -> Arrow:
def roundDate(now: Arrow, unit: str, tz: str = 'UTC', roundDown: bool = True) -> Arrow:
'''
rounds our date object
'''
if roundDown:
now = now.floor(unit)
now = now.floor(unit) # type: ignore[arg-type]
else:
now = now.ceil(unit)
now = now.ceil(unit) # type: ignore[arg-type]
Comment on lines +188 to +190
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arrow types these only with the literal strings they accept. I figured you'd probably prefer keeping the types and checks simpler with a more general str, as any usage of roundDate would also have to comply to only using statically known literals.

if debug: print("roundDate() Now: {0}".format(now))
return now

Expand Down
1 change: 1 addition & 0 deletions datemath/py.typed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Marker file for PEP 561. The python-datemath package uses inline types.
8 changes: 4 additions & 4 deletions requirements-3.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ docutils==0.15.2
freezegun==1.2.2
idna==2.7
linecache2==1.0.0
mypy==1.5.1
mypy-extensions==1.0.0
mypy==1.7.1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd personally bump mypy all the way to latest. 1.7 is the minimum where Unpack is no longer an incomplete feature https://mypy-lang.blogspot.com/2023/11/mypy-17-released.html

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this. mypy is only used in this package to do the type checking so its probably good that its at the latest anyways. That got the tests to pass now

packaging==16.8
pkginfo==1.4.2
Pygments==2.7.4
Expand All @@ -24,9 +23,10 @@ six==1.10.0
tqdm==4.36.1
traceback2==1.4.0
twine==2.0.0
types-python-dateutil==2.8.19.14
types-python-dateutil==2.8.19.20240311
types-setuptools==73.0.0.20240822
types-pytz==2023.3.1.1
typing_extensions==4.7.1
tzdata==2024.1
unittest2==1.1.0
urllib3==1.24.3
webencodings==0.5.1
4 changes: 2 additions & 2 deletions tests-legacy.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import unittest2 as unittest
import unittest
import arrow
from datetime import datetime as pydatetime
from datemath import dm
Expand All @@ -8,7 +8,7 @@
iso8601 = 'YYYY-MM-DDTHH:mm:ssZZ'
class TestDM(unittest.TestCase):

def testParse(self):
def testParse(self) -> None:

# Baisc dates
self.assertEqual(dm('2016.01.02').format(iso8601), '2016-01-02T00:00:00-00:00')
Expand Down
14 changes: 7 additions & 7 deletions tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
class TestDM(unittest.TestCase):


def testBasic(self):
def testBasic(self) -> None:
# Make sure our helpers return the correct objects
self.assertIsInstance(datemath('now'), pydatetime)
self.assertIsInstance(dm('now'), arrow.arrow.Arrow)
Expand All @@ -27,7 +27,7 @@ def testBasic(self):
self.assertEqual(dm('2016-01-02 01:00:00').format(iso8601), '2016-01-02T01:00:00+00:00')


def testRounding(self):
def testRounding(self) -> None:
# Rounding Tests
self.assertEqual(dm('2016-01-01||/d').format('YYYY-MM-DDTHH:mm:ssZZ'), '2016-01-01T00:00:00+00:00')
self.assertEqual(dm('2014-11-18||/y').format('YYYY-MM-DDTHH:mm:ssZZ'), '2014-01-01T00:00:00+00:00')
Expand All @@ -42,7 +42,7 @@ def testRounding(self):
self.assertEqual(dm('2016-01-01||/d', roundDown=False).format('YYYY-MM-DDTHH:mm:ssZZ'), '2016-01-01T23:59:59+00:00')
self.assertEqual(dm('2014-11-18||/y', roundDown=False).format('YYYY-MM-DDTHH:mm:ssZZ'), '2014-12-31T23:59:59+00:00')

def testTimezone(self):
def testTimezone(self) -> None:
# Timezone Tests
with freeze_time(datemath('now/d', tz='US/Pacific')):
self.assertEqual(datemath('now/d', tz='US/Pacific'), pydatetime.now(tz=pytz.timezone("US/Pacific")))
Expand Down Expand Up @@ -75,7 +75,7 @@ def testTimezone(self):
self.assertEqual(datemath('2016-01-01T16:20:00.6+12:00||+2d+1h', tz='US/Eastern'), pydatetime(2016, 1, 3, 17, 20, 0, 600000, tzinfo=tz.tzoffset(None, timedelta(hours=12))))


def testRelativeFormats(self):
def testRelativeFormats(self) -> None:
# relitive formats

# addition
Expand Down Expand Up @@ -135,7 +135,7 @@ def testRelativeFormats(self):
self.assertEqual(dm('now-29d/d').format(iso8601), arrow.utcnow().shift(days=-29).floor('day').format(iso8601))


def testFuture(self):
def testFuture(self) -> None:
# future
self.assertEqual(dm('+1s').format(iso8601), arrow.utcnow().shift(seconds=+1).format(iso8601))
self.assertEqual(dm('+1s+2m+3h').format(iso8601), arrow.utcnow().shift(seconds=+1, minutes=+2, hours=+3).format(iso8601))
Expand All @@ -155,7 +155,7 @@ def testFuture(self):
self.assertEqual(dm('-3w-2d-22h-36s').format(iso8601), arrow.utcnow().shift(weeks=-3, days=-2, hours=-22, seconds=-36).format(iso8601))
self.assertEqual(dm('-6y-3w-2d-22h-36s').format(iso8601), arrow.utcnow().shift(years=-6, weeks=-3, days=-2, hours=-22, seconds=-36).format(iso8601))

def testOther(self):
def testOther(self) -> None:
import datetime
delta = datetime.timedelta(seconds=1)
# datetime objects
Expand All @@ -179,7 +179,7 @@ def testOther(self):
self.assertTrue('Unable to parse epoch timestamps in millis' in str(e))


def testExceptions(self):
def testExceptions(self) -> None:
# Catch invalid timeunits
self.assertRaises(DateMathException, dm, '+1,')
self.assertRaises(DateMathException, dm, '+1.')
Expand Down
Loading