From 4b22d17188c9eae4613891fdcc433ec02f138df8 Mon Sep 17 00:00:00 2001 From: Dima Gerasimov Date: Sun, 24 May 2020 22:20:29 +0100 Subject: [PATCH 1/5] core: add @warn_if_empty decorator --- my/core/__init__.py | 1 + my/core/common.py | 40 ++++++++++++++++++++++++++++++++++++++-- tests/misc.py | 22 ++++++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/my/core/__init__.py b/my/core/__init__.py index 4515235..678df85 100644 --- a/my/core/__init__.py +++ b/my/core/__init__.py @@ -1,4 +1,5 @@ # this file only keeps the most common & critical types/utility functions from .common import PathIsh, Paths, Json from .common import get_files, LazyLogger +from .common import warn_if_empty from .cfg import make_config diff --git a/my/core/common.py b/my/core/common.py index cfadc04..16a730b 100644 --- a/my/core/common.py +++ b/my/core/common.py @@ -3,7 +3,7 @@ from pathlib import Path from datetime import datetime import functools import types -from typing import Union, Callable, Dict, Iterable, TypeVar, Sequence, List, Optional, Any, cast, Tuple +from typing import Union, Callable, Dict, Iterable, TypeVar, Sequence, List, Optional, Any, cast, Tuple, TYPE_CHECKING import warnings # some helper functions @@ -161,7 +161,6 @@ def get_files(pp: Paths, glob: str=DEFAULT_GLOB, sort: bool=True) -> Tuple[Path, # TODO annotate it, perhaps use 'dependent' type (for @doublewrap stuff) -from typing import TYPE_CHECKING if TYPE_CHECKING: from typing import Callable, TypeVar from typing_extensions import Protocol @@ -269,3 +268,40 @@ import re def get_valid_filename(s: str) -> str: s = str(s).strip().replace(' ', '_') return re.sub(r'(?u)[^-\w.]', '', s) + + +from typing import Generic, Sized, Callable + + +def _warn_iterator(it): + emitted = False + for i in it: + yield i + emitted = True + if not emitted: + warnings.warn(f"Function hasn't emitted any data, make sure your config paths are correct") + + +def _warn_iterable(it): + if isinstance(it, Sized): + sz = len(it) + if sz == 0: + warnings.warn(f"Function is returning empty container, make sure your config paths are correct") + return it + else: + return _warn_iterator(it) + + +from functools import wraps +X = TypeVar('X') + +class G(Generic[X], Iterable[X]): + pass + +CC = Callable[[], G] +def warn_if_empty(f: CC) -> CC: + @wraps(f) + def wrapped(*args, **kwargs) -> G[X]: + res = f(*args, **kwargs) + return _warn_iterable(res) + return wrapped diff --git a/tests/misc.py b/tests/misc.py index 40d63a4..a20399e 100644 --- a/tests/misc.py +++ b/tests/misc.py @@ -48,3 +48,25 @@ def prepare(tmp_path: Path): # meh from my.core.error import test_sort_res_by + + +from typing import Iterable, List +import warnings +from my.core import warn_if_empty +def test_warn_if_empty(): + + @warn_if_empty + def nonempty() -> Iterable[str]: + yield 'a' + yield 'aba' + + @warn_if_empty + def empty(arg: str) -> List[str]: + return [] + + with warnings.catch_warnings(record=True) as w: + assert list(nonempty()) == ['a', 'aba'] + assert len(w) == 0 + + assert empty('whatever') == [] + assert len(w) == 1 From e3a71ea6c68ee4c2564a97da8a4fec7e0b55c172 Mon Sep 17 00:00:00 2001 From: Dima Gerasimov Date: Sun, 24 May 2020 23:42:57 +0100 Subject: [PATCH 2/5] my.core: more work on typing @warn_if_empty, extra test --- my/core/common.py | 25 +++++++++++-------------- tests/misc.py | 26 ++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/my/core/common.py b/my/core/common.py index 16a730b..30c2cf0 100644 --- a/my/core/common.py +++ b/my/core/common.py @@ -273,6 +273,7 @@ def get_valid_filename(s: str) -> str: from typing import Generic, Sized, Callable +# X = TypeVar('X') def _warn_iterator(it): emitted = False for i in it: @@ -281,27 +282,23 @@ def _warn_iterator(it): if not emitted: warnings.warn(f"Function hasn't emitted any data, make sure your config paths are correct") - -def _warn_iterable(it): +# todo need to popretly restrict to a container... ugh +C = TypeVar('C') +def _warn_iterable(it: C) -> C: if isinstance(it, Sized): sz = len(it) if sz == 0: warnings.warn(f"Function is returning empty container, make sure your config paths are correct") - return it + return it # type: ignore[return-value] else: return _warn_iterator(it) -from functools import wraps -X = TypeVar('X') - -class G(Generic[X], Iterable[X]): - pass - -CC = Callable[[], G] +CC = TypeVar('CC') def warn_if_empty(f: CC) -> CC: - @wraps(f) - def wrapped(*args, **kwargs) -> G[X]: - res = f(*args, **kwargs) + from functools import wraps + @wraps(f) # type: ignore[arg-type] + def wrapped(*args, **kwargs): + res = f(*args, **kwargs) # type: ignore[call-arg, operator] return _warn_iterable(res) - return wrapped + return wrapped # type: ignore[return-value] diff --git a/tests/misc.py b/tests/misc.py index a20399e..d425196 100644 --- a/tests/misc.py +++ b/tests/misc.py @@ -53,20 +53,38 @@ from my.core.error import test_sort_res_by from typing import Iterable, List import warnings from my.core import warn_if_empty -def test_warn_if_empty(): - +def test_warn_if_empty() -> None: @warn_if_empty def nonempty() -> Iterable[str]: yield 'a' yield 'aba' @warn_if_empty - def empty(arg: str) -> List[str]: + def empty() -> List[str]: return [] with warnings.catch_warnings(record=True) as w: assert list(nonempty()) == ['a', 'aba'] assert len(w) == 0 - assert empty('whatever') == [] + eee = empty() + assert eee == [] assert len(w) == 1 + + +def test_warn_iterable() -> None: + from my.core.common import _warn_iterable + i1: List[str] = ['a', 'b'] + i2: Iterable[int] = iter([1, 2, 3]) + # reveal_type(i1) + # reveal_type(i2) + x1 = _warn_iterable(i1) + x2 = _warn_iterable(i2) + # reveal_type(x1) + # reveal_type(x2) + with warnings.catch_warnings(record=True) as w: + assert x1 is i1 # should be unchanged! + assert len(w) == 0 + + assert list(x2) == [1, 2, 3] + assert len(w) == 0 From 616ffb457ef73ef0614445f217f4e335ed758cf1 Mon Sep 17 00:00:00 2001 From: Dima Gerasimov Date: Mon, 25 May 2020 00:22:29 +0100 Subject: [PATCH 3/5] core: user overloads to type @warn_if_empty properly.. --- my/core/common.py | 32 ++++++++++++++++++++++++-------- tests/misc.py | 11 +++++++++++ 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/my/core/common.py b/my/core/common.py index 30c2cf0..10599f5 100644 --- a/my/core/common.py +++ b/my/core/common.py @@ -282,9 +282,22 @@ def _warn_iterator(it): if not emitted: warnings.warn(f"Function hasn't emitted any data, make sure your config paths are correct") -# todo need to popretly restrict to a container... ugh -C = TypeVar('C') -def _warn_iterable(it: C) -> C: + +# TODO ugh, so I want to express something like: +# X = TypeVar('X') +# C = TypeVar('C', bound=Iterable[X]) +# _warn_iterable(it: C) -> C +# but apparently I can't??? ugh. +# https://github.com/python/typing/issues/548 +# I guess for now overloads are fine... + +from typing import overload +X = TypeVar('X') +@overload +def _warn_iterable(it: List[X] ) -> List[X] : ... +@overload +def _warn_iterable(it: Iterable[X]) -> Iterable[X]: ... +def _warn_iterable(it): if isinstance(it, Sized): sz = len(it) if sz == 0: @@ -294,11 +307,14 @@ def _warn_iterable(it: C) -> C: return _warn_iterator(it) -CC = TypeVar('CC') -def warn_if_empty(f: CC) -> CC: +@overload +def warn_if_empty(f: Callable[[], List[X]] ) -> Callable[[], List[X]] : ... +@overload +def warn_if_empty(f: Callable[[], Iterable[X]]) -> Callable[[], Iterable[X]]: ... +def warn_if_empty(f): from functools import wraps - @wraps(f) # type: ignore[arg-type] + @wraps(f) def wrapped(*args, **kwargs): - res = f(*args, **kwargs) # type: ignore[call-arg, operator] + res = f(*args, **kwargs) return _warn_iterable(res) - return wrapped # type: ignore[return-value] + return wrapped diff --git a/tests/misc.py b/tests/misc.py index d425196..e503eb6 100644 --- a/tests/misc.py +++ b/tests/misc.py @@ -63,6 +63,15 @@ def test_warn_if_empty() -> None: def empty() -> List[str]: return [] + # should be rejected by mypy! + # todo how to actually test it? + # @warn_if_empty + # def baad() -> float: + # return 0.00 + + # reveal_type(nonempty) + # reveal_type(empty) + with warnings.catch_warnings(record=True) as w: assert list(nonempty()) == ['a', 'aba'] assert len(w) == 0 @@ -80,6 +89,8 @@ def test_warn_iterable() -> None: # reveal_type(i2) x1 = _warn_iterable(i1) x2 = _warn_iterable(i2) + # vvvv this should be flagged by mypy + # _warn_iterable(123) # reveal_type(x1) # reveal_type(x2) with warnings.catch_warnings(record=True) as w: From 216944b3cd4b2f9e4ce47c9f62a8cd400f8d2d51 Mon Sep 17 00:00:00 2001 From: Dima Gerasimov Date: Mon, 25 May 2020 00:56:03 +0100 Subject: [PATCH 4/5] core: improvements for warnings, twitter/rss: try using @warn_if_empty --- my/core/common.py | 31 +++++++++++++++++++------------ my/rss/all.py | 9 +++++++-- my/rss/common.py | 5 +++++ my/twitter/all.py | 5 ++--- my/twitter/common.py | 2 ++ 5 files changed, 35 insertions(+), 17 deletions(-) diff --git a/my/core/common.py b/my/core/common.py index 10599f5..0c7571e 100644 --- a/my/core/common.py +++ b/my/core/common.py @@ -130,6 +130,11 @@ def get_files(pp: Paths, glob: str=DEFAULT_GLOB, sort: bool=True) -> Tuple[Path, else: sources.extend(map(Path, pp)) + def caller() -> str: + import traceback + # TODO ugh. very flaky... -3 because [, get_files(), ] + return traceback.extract_stack()[-3].filename + paths: List[Path] = [] for src in sources: if src.parts[0] == '~': @@ -141,7 +146,7 @@ def get_files(pp: Paths, glob: str=DEFAULT_GLOB, sort: bool=True) -> Tuple[Path, ss = str(src) if '*' in ss: if glob != DEFAULT_GLOB: - warnings.warn(f"Treating {ss} as glob path. Explicit glob={glob} argument is ignored!") + warnings.warn(f"{caller()}: treating {ss} as glob path. Explicit glob={glob} argument is ignored!") paths.extend(map(Path, do_glob(ss))) else: if not src.is_file(): @@ -154,8 +159,10 @@ def get_files(pp: Paths, glob: str=DEFAULT_GLOB, sort: bool=True) -> Tuple[Path, if len(paths) == 0: # todo make it conditionally defensive based on some global settings - # todo stacktrace? - warnings.warn(f'No paths were matched against {paths}. This might result in missing data.') + # TODO not sure about using warnings module for this + import traceback + warnings.warn(f'{caller()}: no paths were matched against {paths}. This might result in missing data.') + traceback.print_stack() return tuple(paths) @@ -274,13 +281,13 @@ from typing import Generic, Sized, Callable # X = TypeVar('X') -def _warn_iterator(it): +def _warn_iterator(it, f: Any=None): emitted = False for i in it: yield i emitted = True if not emitted: - warnings.warn(f"Function hasn't emitted any data, make sure your config paths are correct") + warnings.warn(f"Function {f} didn't emit any data, make sure your config paths are correct") # TODO ugh, so I want to express something like: @@ -294,17 +301,17 @@ def _warn_iterator(it): from typing import overload X = TypeVar('X') @overload -def _warn_iterable(it: List[X] ) -> List[X] : ... +def _warn_iterable(it: List[X] , f: Any=None) -> List[X] : ... @overload -def _warn_iterable(it: Iterable[X]) -> Iterable[X]: ... -def _warn_iterable(it): +def _warn_iterable(it: Iterable[X], f: Any=None) -> Iterable[X]: ... +def _warn_iterable(it, f=None): if isinstance(it, Sized): sz = len(it) if sz == 0: - warnings.warn(f"Function is returning empty container, make sure your config paths are correct") - return it # type: ignore[return-value] + warnings.warn(f"Function {f} returned empty container, make sure your config paths are correct") + return it else: - return _warn_iterator(it) + return _warn_iterator(it, f=f) @overload @@ -316,5 +323,5 @@ def warn_if_empty(f): @wraps(f) def wrapped(*args, **kwargs): res = f(*args, **kwargs) - return _warn_iterable(res) + return _warn_iterable(res, f=f) return wrapped diff --git a/my/rss/all.py b/my/rss/all.py index 90f5efa..61f9fab 100644 --- a/my/rss/all.py +++ b/my/rss/all.py @@ -1,11 +1,16 @@ ''' Unified RSS data, merged from different services I used historically ''' +# NOTE: you can comment out the sources you're not using +from . import feedbin, feedly + from typing import Iterable from .common import Subscription, compute_subscriptions def subscriptions() -> Iterable[Subscription]: - from . import feedbin, feedly # TODO google reader? - yield from compute_subscriptions(feedbin.states(), feedly.states()) + yield from compute_subscriptions( + feedbin.states(), + feedly .states(), + ) diff --git a/my/rss/common.py b/my/rss/common.py index 3dc761c..9aa5ed8 100644 --- a/my/rss/common.py +++ b/my/rss/common.py @@ -17,6 +17,8 @@ from typing import Iterable, Tuple, Sequence SubscriptionState = Tuple[datetime, Sequence[Subscription]] +from ..core import warn_if_empty +@warn_if_empty def compute_subscriptions(*sources: Iterable[SubscriptionState]) -> List[Subscription]: """ Keeps track of everything I ever subscribed to. @@ -34,6 +36,9 @@ def compute_subscriptions(*sources: Iterable[SubscriptionState]) -> List[Subscri if feed.url not in by_url: by_url[feed.url] = feed + if len(states) == 0: + return [] + _, last_state = max(states, key=lambda x: x[0]) last_urls = {f.url for f in last_state} diff --git a/my/twitter/all.py b/my/twitter/all.py index acb59a2..5c8103c 100644 --- a/my/twitter/all.py +++ b/my/twitter/all.py @@ -3,11 +3,9 @@ Unified Twitter data (merged from the archive and periodic updates) """ # NOTE: you can comment out the sources you don't need - - from . import twint, archive -from .common import merge_tweets +from .common import merge_tweets def tweets(): yield from merge_tweets( @@ -15,6 +13,7 @@ def tweets(): archive.tweets(), ) +from .common import merge_tweets def likes(): yield from merge_tweets( diff --git a/my/twitter/common.py b/my/twitter/common.py index 1bf36f0..ecfaea3 100644 --- a/my/twitter/common.py +++ b/my/twitter/common.py @@ -2,7 +2,9 @@ from itertools import chain from more_itertools import unique_everseen +from ..core import warn_if_empty +@warn_if_empty def merge_tweets(*sources): yield from unique_everseen( chain(*sources), From 248e48dc308c00e58becc47a6e0f149454ee21ff Mon Sep 17 00:00:00 2001 From: Dima Gerasimov Date: Mon, 25 May 2020 01:23:30 +0100 Subject: [PATCH 5/5] core: improve types for warn_if_empty ok, works with this advice https://github.com/python/mypy/issues/1927 + overloads --- my/core/common.py | 13 ++++++++++--- tests/misc.py | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/my/core/common.py b/my/core/common.py index 0c7571e..64e7b23 100644 --- a/my/core/common.py +++ b/my/core/common.py @@ -314,14 +314,21 @@ def _warn_iterable(it, f=None): return _warn_iterator(it, f=f) +# ok, this seems to work... +# https://github.com/python/mypy/issues/1927#issue-167100413 +FL = TypeVar('FL', bound=Callable[..., List]) +FI = TypeVar('FI', bound=Callable[..., Iterable]) + @overload -def warn_if_empty(f: Callable[[], List[X]] ) -> Callable[[], List[X]] : ... +def warn_if_empty(f: FL) -> FL: ... @overload -def warn_if_empty(f: Callable[[], Iterable[X]]) -> Callable[[], Iterable[X]]: ... +def warn_if_empty(f: FI) -> FI: ... + + def warn_if_empty(f): from functools import wraps @wraps(f) def wrapped(*args, **kwargs): res = f(*args, **kwargs) return _warn_iterable(res, f=f) - return wrapped + return wrapped # type: ignore diff --git a/tests/misc.py b/tests/misc.py index e503eb6..8930851 100644 --- a/tests/misc.py +++ b/tests/misc.py @@ -60,7 +60,7 @@ def test_warn_if_empty() -> None: yield 'aba' @warn_if_empty - def empty() -> List[str]: + def empty() -> List[int]: return [] # should be rejected by mypy!