From 4b92fd0398450976201efa0c6b211da808c95404 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 12:07:15 +0200 Subject: [PATCH 01/14] Remove Python 3.5 from list of supported versions --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index 22e16370..209cde33 100644 --- a/setup.py +++ b/setup.py @@ -65,7 +65,6 @@ def parse_requirements(requirements, ignore=('setuptools',)): "Operating System :: Microsoft :: Windows", "Operating System :: POSIX", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.5", "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", From 8000bd20ec2a36d0fa8d8e484c613cebf6bd85a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 18:39:18 +0200 Subject: [PATCH 02/14] Drop py35 from tox envlist --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index f500c5e2..54209e79 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = flake8,py35,py36,py37,py38,py39 +envlist = flake8,py36,py37,py38,py39 skip_missing_interpreters = True [testenv] From 20f0b3446a123beeb426ebff56d59fdfb3340066 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 12:14:47 +0200 Subject: [PATCH 03/14] Fix return type annotation --- watson/cli.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/watson/cli.py b/watson/cli.py index 305d1af5..a64f85ee 100644 --- a/watson/cli.py +++ b/watson/cli.py @@ -5,6 +5,7 @@ import os from dateutil import tz from functools import reduce, wraps +from typing import Optional import arrow import click @@ -85,7 +86,7 @@ def local_tz_info() -> datetime.tzinfo: class DateTimeParamType(click.ParamType): name = 'datetime' - def convert(self, value, param, ctx) -> arrow: + def convert(self, value, param, ctx) -> Optional[arrow.Arrow]: if value: date = self._parse_multiformat(value) if date is None: @@ -107,7 +108,7 @@ def convert(self, value, param, ctx) -> arrow: start_time=date, week_start=week_start) return date - def _parse_multiformat(self, value) -> arrow: + def _parse_multiformat(self, value) -> Optional[arrow.Arrow]: date = None for fmt in (None, 'HH:mm:ss', 'HH:mm'): try: From e4e36cb8c12a9ab86690847a6bd0be17dcc1cfbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 12:22:25 +0200 Subject: [PATCH 04/14] Add type hints to apply_weekday_offset The added type hints are required to prevent a mypy complaint later on. --- watson/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/watson/utils.py b/watson/utils.py index c276860d..18aa253e 100644 --- a/watson/utils.py +++ b/watson/utils.py @@ -186,7 +186,7 @@ def get_start_time_for_period(period): return start_time -def apply_weekday_offset(start_time, week_start): +def apply_weekday_offset(start_time: arrow.Arrow, week_start) -> arrow.Arrow: """ Apply the offset required to move the start date `start_time` of a week starting on Monday to that of a week starting on `week_start`. From 911b53947235acd97814649aab908acee7ea47e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 12:23:32 +0200 Subject: [PATCH 05/14] Drop checking if `value` evaluates to True The documentation of click.ParamType states that `ParamType.convert` will never get a parameter `value` that is `None` [1]. Therefore, the entire branching can be removed. With it, the implicit return of None is removed too. [1]: https://click.palletsprojects.com/en/8.0.x/api/#click.ParamType.convert --- watson/cli.py | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/watson/cli.py b/watson/cli.py index a64f85ee..7c37f90a 100644 --- a/watson/cli.py +++ b/watson/cli.py @@ -86,27 +86,26 @@ def local_tz_info() -> datetime.tzinfo: class DateTimeParamType(click.ParamType): name = 'datetime' - def convert(self, value, param, ctx) -> Optional[arrow.Arrow]: - if value: - date = self._parse_multiformat(value) - if date is None: - raise click.UsageError( - "Could not match value '{}' to any supported date format" - .format(value) - ) - # When we parse a date, we want to parse it in the timezone - # expected by the user, so that midnight is midnight in the local - # timezone, or respect the TZ environment variable not in UTC. - # Cf issue #16. - date = date.replace(tzinfo=local_tz_info()) - # Add an offset to match the week beginning specified in the - # configuration - if param.name == "week": - week_start = ctx.obj.config.get( - "options", "week_start", "monday") - date = apply_weekday_offset( - start_time=date, week_start=week_start) - return date + def convert(self, value, param, ctx) -> arrow.Arrow: + date = self._parse_multiformat(value) + if date is None: + raise click.UsageError( + "Could not match value '{}' to any supported date format" + .format(value) + ) + # When we parse a date, we want to parse it in the timezone + # expected by the user, so that midnight is midnight in the local + # timezone, or respect the TZ environment variable not in UTC. + # Cf issue #16. + date = date.replace(tzinfo=local_tz_info()) + # Add an offset to match the week beginning specified in the + # configuration + if param.name == "week": + week_start = ctx.obj.config.get( + "options", "week_start", "monday") + date = apply_weekday_offset( + start_time=date, week_start=week_start) + return date def _parse_multiformat(self, value) -> Optional[arrow.Arrow]: date = None From abdf5f4edf573405c2fb75a01d916c75d3aaf85a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 18:08:19 +0200 Subject: [PATCH 06/14] Add some innocent type annotations --- tests/test_watson.py | 2 +- watson/cli.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_watson.py b/tests/test_watson.py index b44ee172..fc038b79 100644 --- a/tests/test_watson.py +++ b/tests/test_watson.py @@ -73,7 +73,7 @@ def test_current_with_empty_given_state(config_dir, mocker): assert watson.current == {} -def test_current_as_running_frame(watson): +def test_current_as_running_frame(watson) -> None: """ Ensures frame can be created without a stop date. Catches #417: editing task in progress throws an exception diff --git a/watson/cli.py b/watson/cli.py index 7c37f90a..382cd8f1 100644 --- a/watson/cli.py +++ b/watson/cli.py @@ -1256,7 +1256,9 @@ def add(watson, args, from_, to, confirm_new_project, confirm_new_tag): @click.argument('id', required=False, autocompletion=get_frames) @click.pass_obj @catch_watson_error -def edit(watson, confirm_new_project, confirm_new_tag, id): +def edit( + watson, confirm_new_project: bool, confirm_new_tag: bool, id: Optional[str] +): """ Edit a frame. From 706664617961feb5a368890c68cbcac682a4d9b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 18:09:14 +0200 Subject: [PATCH 07/14] Add type annotations to Frame constructors A bit of guessing was involved. In one case a potential bug was uncovered: If the raw string for `stop` was empty, the initialisation of the stop time would have been skipped. This is fixed right ahead. --- watson/frames.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/watson/frames.py b/watson/frames.py index 0e511578..a731eaa4 100644 --- a/watson/frames.py +++ b/watson/frames.py @@ -1,19 +1,29 @@ import uuid +from typing import List, Optional, Union import arrow from collections import namedtuple +TimeType = Union[str, arrow.Arrow] HEADERS = ('start', 'stop', 'project', 'id', 'tags', 'updated_at') class Frame(namedtuple('Frame', HEADERS)): - def __new__(cls, start, stop, project, id, tags=None, updated_at=None,): + def __new__( + cls, + start: TimeType, + stop: Optional[TimeType], + project: str, + id: Optional[str], + tags: Optional[List[str]] = None, + updated_at: Optional[TimeType] = None, + ) -> "Frame": try: if not isinstance(start, arrow.Arrow): start = arrow.get(start) - if stop and not isinstance(stop, arrow.Arrow): + if stop is not None and not isinstance(stop, arrow.Arrow): stop = arrow.get(stop) if updated_at is None: @@ -138,8 +148,15 @@ def add(self, *args, **kwargs): self._rows.append(frame) return frame - def new_frame(self, project, start, stop, tags=None, id=None, - updated_at=None): + def new_frame( + self, + project: str, + start: TimeType, + stop: Optional[TimeType], + tags: Optional[List[str]] = None, + id: Optional[str] = None, + updated_at: Optional[TimeType] = None, + ) -> Frame: if not id: id = uuid.uuid4().hex return Frame(start, stop, project, id, tags=tags, From fa45e9636aa08c9b84e81e2ad8ef75103297ad35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 18:11:14 +0200 Subject: [PATCH 08/14] Switch to non-default constructor A classmethod `make_new` is introduced and used throughout the code. This prepares rewriting the class to a typed NamedTuple. --- tests/test_watson.py | 8 ++++++- watson/cli.py | 9 ++++++-- watson/frames.py | 54 +++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 65 insertions(+), 6 deletions(-) diff --git a/tests/test_watson.py b/tests/test_watson.py index fc038b79..55e741e3 100644 --- a/tests/test_watson.py +++ b/tests/test_watson.py @@ -81,7 +81,13 @@ def test_current_as_running_frame(watson) -> None: watson.start('foo', tags=['A', 'B']) cur = watson.current - frame = Frame(cur['start'], None, cur['project'], None, cur['tags']) + frame = Frame.make_new( + start=cur['start'], + stop=None, + project=cur['project'], + id=None, + tags=cur['tags'], + ) assert frame.stop is None assert frame.project == 'foo' diff --git a/watson/cli.py b/watson/cli.py index 382cd8f1..71680a9d 100644 --- a/watson/cli.py +++ b/watson/cli.py @@ -1282,8 +1282,13 @@ def edit( frame = get_frame_from_argument(watson, id) id = frame.id elif watson.is_started: - frame = Frame(watson.current['start'], None, watson.current['project'], - None, watson.current['tags']) + frame = Frame.make_new( + start=watson.current['start'], + stop=None, + project=watson.current['project'], + id=None, + tags=watson.current['tags'] + ) elif watson.frames: frame = watson.frames[-1] id = frame.id diff --git a/watson/frames.py b/watson/frames.py index a731eaa4..f4403d99 100644 --- a/watson/frames.py +++ b/watson/frames.py @@ -46,6 +46,48 @@ def __new__( cls, start, stop, project, id, tags, updated_at ) + @classmethod + def make_new( + cls, + start: TimeType, + stop: Optional[TimeType], + project: str, + id: Optional[str], + tags: Optional[List[str]] = None, + updated_at: Optional[TimeType] = None, + ) -> "Frame": + try: + if not isinstance(start, arrow.Arrow): + start = arrow.get(start) + + if stop is not None and not isinstance(stop, arrow.Arrow): + stop = arrow.get(stop) + + if updated_at is None: + updated_at = arrow.utcnow() + elif not isinstance(updated_at, arrow.Arrow): + updated_at = arrow.get(updated_at) + except (ValueError, TypeError) as e: + from .watson import WatsonError + raise WatsonError("Error converting date: {}".format(e)) + + start = start.to('local') + + if stop: + stop = stop.to('local') + + if tags is None: + tags = [] + + return cls( + start=start, + stop=stop, + project=project, + id=id, + tags=tags, + updated_at=updated_at, + ) + def dump(self): start = self.start.to('utc').int_timestamp stop = self.stop.to('utc').int_timestamp if self.stop else None @@ -88,7 +130,7 @@ def __init__(self, frames=None): if not frames: frames = [] - rows = [Frame(*frame) for frame in frames] + rows = [Frame.make_new(*frame) for frame in frames] self._rows = rows self.changed = False @@ -159,8 +201,14 @@ def new_frame( ) -> Frame: if not id: id = uuid.uuid4().hex - return Frame(start, stop, project, id, tags=tags, - updated_at=updated_at) + return Frame.make_new( + start=start, + stop=stop, + project=project, + id=id, + tags=tags, + updated_at=updated_at, + ) def dump(self): return tuple(frame.dump() for frame in self._rows) From cb5040fb177c9c867d2ade6295f7c569f9c41b0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 18:20:14 +0200 Subject: [PATCH 09/14] Use typing.NamedTuple Using typing.NamedTuple has plenty of advantages over collections.namedtuple. The most important is that now HEADERS is not required anymore for class construction. --- watson/frames.py | 47 ++++++++--------------------------------------- 1 file changed, 8 insertions(+), 39 deletions(-) diff --git a/watson/frames.py b/watson/frames.py index f4403d99..12773926 100644 --- a/watson/frames.py +++ b/watson/frames.py @@ -1,50 +1,19 @@ import uuid -from typing import List, Optional, Union +from typing import List, NamedTuple, Optional, Union import arrow -from collections import namedtuple - TimeType = Union[str, arrow.Arrow] HEADERS = ('start', 'stop', 'project', 'id', 'tags', 'updated_at') -class Frame(namedtuple('Frame', HEADERS)): - def __new__( - cls, - start: TimeType, - stop: Optional[TimeType], - project: str, - id: Optional[str], - tags: Optional[List[str]] = None, - updated_at: Optional[TimeType] = None, - ) -> "Frame": - try: - if not isinstance(start, arrow.Arrow): - start = arrow.get(start) - - if stop is not None and not isinstance(stop, arrow.Arrow): - stop = arrow.get(stop) - - if updated_at is None: - updated_at = arrow.utcnow() - elif not isinstance(updated_at, arrow.Arrow): - updated_at = arrow.get(updated_at) - except (ValueError, TypeError) as e: - from .watson import WatsonError - raise WatsonError("Error converting date: {}".format(e)) - - start = start.to('local') - - if stop: - stop = stop.to('local') - - if tags is None: - tags = [] - - return super(Frame, cls).__new__( - cls, start, stop, project, id, tags, updated_at - ) +class Frame(NamedTuple): + start: arrow.Arrow + stop: Optional[arrow.Arrow] + project: str + id: Optional[str] + tags: List[str] + updated_at: arrow.Arrow @classmethod def make_new( From df7dd16d557342767336270eda2c82b49a6a29cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 18:22:29 +0200 Subject: [PATCH 10/14] Drop new-class style class definition Its relevant only in Python 2.7. --- watson/frames.py | 4 ++-- watson/watson.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/watson/frames.py b/watson/frames.py index 12773926..885f38b9 100644 --- a/watson/frames.py +++ b/watson/frames.py @@ -81,7 +81,7 @@ def __gte__(self, other): return self.start >= other.start -class Span(object): +class Span: def __init__(self, start, stop, timeframe='day'): self.timeframe = timeframe self.start = start.floor(self.timeframe) @@ -94,7 +94,7 @@ def __contains__(self, frame): return frame.start >= self.start and frame.stop <= self.stop -class Frames(object): +class Frames: def __init__(self, frames=None): if not frames: frames = [] diff --git a/watson/watson.py b/watson/watson.py index 7ab7da12..cc5a8cf1 100644 --- a/watson/watson.py +++ b/watson/watson.py @@ -22,7 +22,7 @@ class ConfigurationError(CFGParserError, WatsonError): pass -class Watson(object): +class Watson: def __init__(self, **kwargs): """ :param frames: If given, should be a list representing the From ed30d6441db58b562f39a091c517814449ed7522 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 18:28:59 +0200 Subject: [PATCH 11/14] Drop usage of HEADERS entirely --- watson/frames.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/watson/frames.py b/watson/frames.py index 885f38b9..67c10822 100644 --- a/watson/frames.py +++ b/watson/frames.py @@ -4,7 +4,6 @@ import arrow TimeType = Union[str, arrow.Arrow] -HEADERS = ('start', 'stop', 'project', 'id', 'tags', 'updated_at') class Frame(NamedTuple): @@ -108,7 +107,8 @@ def __len__(self): return len(self._rows) def __getitem__(self, key): - if key in HEADERS: + attributes = Frame.__annotations__.keys() + if key in attributes: return tuple(self._get_col(key)) elif isinstance(key, int): return self._rows[key] @@ -149,9 +149,8 @@ def _get_index_by_id(self, id): raise KeyError("Frame with id {} not found.".format(id)) def _get_col(self, col): - index = HEADERS.index(col) for row in self._rows: - yield row[index] + yield getattr(row, col) def add(self, *args, **kwargs): self.changed = True From 68493b4cc83384f3837782303cbd6e5b87ed9c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 18:37:38 +0200 Subject: [PATCH 12/14] Add more innocent type hints to signatures --- watson/frames.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/watson/frames.py b/watson/frames.py index 67c10822..178a40ee 100644 --- a/watson/frames.py +++ b/watson/frames.py @@ -1,9 +1,12 @@ import uuid -from typing import List, NamedTuple, Optional, Union +from typing import Iterable, List, NamedTuple, Optional, Union import arrow TimeType = Union[str, arrow.Arrow] +FrameColumnTypes = Union[ + arrow.Arrow, Optional[arrow.Arrow], str, Optional[str], List[str] +] class Frame(NamedTuple): @@ -64,7 +67,7 @@ def dump(self): return (start, stop, self.project, self.id, self.tags, updated_at) @property - def day(self): + def day(self) -> arrow.Arrow: return self.start.floor('day') def __lt__(self, other): @@ -81,20 +84,20 @@ def __gte__(self, other): class Span: - def __init__(self, start, stop, timeframe='day'): + def __init__(self, start: arrow.Arrow, stop: arrow.Arrow, timeframe='day'): self.timeframe = timeframe self.start = start.floor(self.timeframe) self.stop = stop.ceil(self.timeframe) - def overlaps(self, frame): + def overlaps(self, frame: Frame) -> bool: return frame.start <= self.stop and frame.stop >= self.start - def __contains__(self, frame): + def __contains__(self, frame: Frame) -> bool: return frame.start >= self.start and frame.stop <= self.stop class Frames: - def __init__(self, frames=None): + def __init__(self, frames: Optional[List[Frame]] = None): if not frames: frames = [] @@ -103,7 +106,7 @@ def __init__(self, frames=None): self.changed = False - def __len__(self): + def __len__(self) -> int: return len(self._rows) def __getitem__(self, key): @@ -140,7 +143,7 @@ def __delitem__(self, key): else: del self._rows[self._get_index_by_id(key)] - def _get_index_by_id(self, id): + def _get_index_by_id(self, id: str) -> int: try: return next( i for i, v in enumerate(self['id']) if v.startswith(id) @@ -148,7 +151,7 @@ def _get_index_by_id(self, id): except StopIteration: raise KeyError("Frame with id {} not found.".format(id)) - def _get_col(self, col): + def _get_col(self, col: str) -> Iterable[FrameColumnTypes]: for row in self._rows: yield getattr(row, col) @@ -216,5 +219,5 @@ def filter( stop = span.stop if frame.stop > span.stop else frame.stop yield frame._replace(start=start, stop=stop) - def span(self, start, stop): + def span(self, start: arrow.Arrow, stop: arrow.Arrow) -> Span: return Span(start, stop) From e48686677361f9b4c8d58a2be70283187bbb6ae0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 18:45:07 +0200 Subject: [PATCH 13/14] Add mypy to tox configuration Now that all very obvious type problems are resolved, mypy can be used in the static analysis part. --- setup.cfg | 6 ++++++ tox.ini | 7 ++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index d662cb96..3c528c10 100644 --- a/setup.cfg +++ b/setup.cfg @@ -3,3 +3,9 @@ universal=1 [aliases] test=pytest + +[mypy] +ignore_missing_imports = True +warn_return_any = True +warn_unreachable = True +warn_unused_ignores = True diff --git a/tox.ini b/tox.ini index 54209e79..d626d5cb 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = flake8,py36,py37,py38,py39 +envlist = flake8,mypy,py36,py37,py38,py39 skip_missing_interpreters = True [testenv] @@ -15,3 +15,8 @@ usedevelop = True [testenv:flake8] deps = flake8 commands = flake8 --show-source watson/ tests/ scripts/ + + +[testenv:mypy] +deps = mypy==0.812 +commands = mypy watson/ tests/ scripts/ From 6276cd3af798236d360a8f9120ed3ae528490d54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20G=C3=B6rner?= <5477952+MaxG87@users.noreply.github.com> Date: Fri, 4 Jun 2021 18:58:07 +0200 Subject: [PATCH 14/14] Add mypy to TravisCI --- .travis.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.travis.yml b/.travis.yml index 88bee9a6..916f69a3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,6 +22,22 @@ matrix: os: linux dist: xenial env: TOXENV=py39 + - python: 3.6 + os: linux + dist: trusty + env: TOXENV=mypy + - python: 3.7 + os: linux + dist: xenial + env: TOXENV=mypy + - python: 3.8 + os: linux + dist: xenial + env: TOXENV=mypy + - python: 3.9 + os: linux + dist: xenial + env: TOXENV=mypy install: - pip install tox