Skip to content
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

test: implement type hints #906

Merged
merged 28 commits into from
Feb 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
6511dcb
test: reconcile main with mypy changes
kuwv Jan 20, 2023
f2c6cc9
test: fix tests with mypy runners
kuwv Jan 20, 2023
dcaa093
test: update mypy for env and config
kuwv Jan 20, 2023
504525f
test: update mypy for env and config
kuwv Jan 20, 2023
6e2ec91
test: add type-hints
kuwv Jan 26, 2023
82b2649
test: Update dev-requirements.txt
kuwv Jan 28, 2023
72b337a
test: implement additional typing
kuwv Jan 29, 2023
8e0d612
test: completed mypy integration
kuwv Jan 29, 2023
e9bdb6e
test: add pyproject extensions and config
kuwv Jan 29, 2023
efef3ce
test: cleanup mypy config
kuwv Jan 29, 2023
56ddd30
test: cleanup mypy config
kuwv Jan 29, 2023
6b08c58
test: attempt local typing import
kuwv Jan 29, 2023
629155d
test: attempt local typing import
kuwv Jan 29, 2023
a9e5617
style: blacken
kuwv Jan 29, 2023
202a1ef
refactor: remove py2 cruft
kuwv Jan 30, 2023
85e9cf6
Update argument.py
kuwv Jan 30, 2023
cfdd42d
refactor: revert import changes
kuwv Jan 30, 2023
d6fd2b9
refactor: remove cruft
kuwv Jan 30, 2023
7c20e5a
test: resolve 906 threads
kuwv Feb 4, 2023
dc76dab
test: resolve 906 threads
kuwv Feb 4, 2023
f24917c
test: resolve 906 threads
kuwv Feb 4, 2023
38d0a91
test: resolve 906 threads
kuwv Feb 5, 2023
2ee76a7
test: update changelog for 376 bug
kuwv Feb 5, 2023
1f3e5a4
test: update changelog for 376 bug
kuwv Feb 5, 2023
70e9d93
refactor: set debug without globals injection
kuwv Feb 5, 2023
0a23f95
refactor: use userlist
kuwv Feb 5, 2023
0b6a448
test: add sams suggestions
kuwv Feb 11, 2023
f24e290
test: add sams suggestions
kuwv Feb 11, 2023
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
1 change: 0 additions & 1 deletion .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,3 @@
exclude = invoke/vendor,sites,.git,build,dist,alt_env,appveyor
ignore = E124,E125,E128,E261,E301,E302,E303,E306,W503,E731
max-line-length = 79

1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ src/
htmlcov
coverage.xml
.cache
.mypy_cache/
4 changes: 4 additions & 0 deletions dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,7 @@ black>=22.8,<22.9
setuptools>56
# Debuggery
icecream>=2.1
# typing
mypy==0.971
typed-ast==1.5.4
types-PyYAML==6.0.12.4
6 changes: 4 additions & 2 deletions invoke/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import Any, Optional

from ._version import __version_info__, __version__ # noqa
from .collection import Collection # noqa
from .config import Config # noqa
Expand Down Expand Up @@ -29,7 +31,7 @@
from .watchers import FailingResponder, Responder, StreamWatcher # noqa


def run(command, **kwargs):
def run(command: str, **kwargs: Any) -> Optional[Result]:
"""
Run ``command`` in a subprocess and return a `.Result` object.

Expand All @@ -48,7 +50,7 @@ def run(command, **kwargs):
return Context().run(command, **kwargs)


def sudo(command, **kwargs):
def sudo(command: str, **kwargs: Any) -> Optional[Result]:
"""
Run ``command`` in a ``sudo`` subprocess and return a `.Result` object.

Expand Down
130 changes: 74 additions & 56 deletions invoke/collection.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import copy
import types
from types import ModuleType
from typing import Any, Callable, Dict, List, Optional, Tuple

from .util import Lexicon, helpline

Expand All @@ -15,7 +16,7 @@ class Collection:
.. versionadded:: 1.0
"""

def __init__(self, *args, **kwargs):
def __init__(self, *args: Any, **kwargs: Any) -> None:
"""
Create a new task collection/namespace.

Expand Down Expand Up @@ -92,67 +93,64 @@ def __init__(self, *args, **kwargs):
# Initialize
self.tasks = Lexicon()
self.collections = Lexicon()
self.default = None
self.default: Optional[str] = None
self.name = None
self._configuration = {}
self._configuration: Dict[str, Any] = {}
# Specific kwargs if applicable
self.loaded_from = kwargs.pop("loaded_from", None)
self.auto_dash_names = kwargs.pop("auto_dash_names", None)
# splat-kwargs version of default value (auto_dash_names=True)
if self.auto_dash_names is None:
self.auto_dash_names = True
# Name if applicable
args = list(args)
if args and isinstance(args[0], str):
self.name = self.transform(args.pop(0))
_args = list(args)
if _args and isinstance(args[0], str):
self.name = self.transform(_args.pop(0))
# Dispatch args/kwargs
for arg in args:
for arg in _args:
self._add_object(arg)
# Dispatch kwargs
for name, obj in kwargs.items():
self._add_object(obj, name)

def _add_object(self, obj, name=None):
def _add_object(self, obj: Any, name: Optional[str] = None) -> None:
method: Callable
if isinstance(obj, Task):
method = self.add_task
elif isinstance(obj, (Collection, types.ModuleType)):
elif isinstance(obj, (Collection, ModuleType)):
method = self.add_collection
else:
raise TypeError("No idea how to insert {!r}!".format(type(obj)))
return method(obj, name=name)
method(obj, name=name)

def __repr__(self):
def __repr__(self) -> str:
task_names = list(self.tasks.keys())
collections = ["{}...".format(x) for x in self.collections.keys()]
return "<Collection {!r}: {}>".format(
self.name, ", ".join(sorted(task_names) + sorted(collections))
)

def __eq__(self, other):
return (
self.name == other.name
and self.tasks == other.tasks
and self.collections == other.collections
)

def __ne__(self, other):
return not self == other

def __nonzero__(self):
return self.__bool__()
def __eq__(self, other: object) -> bool:
if isinstance(other, Collection):
kuwv marked this conversation as resolved.
Show resolved Hide resolved
return (
self.name == other.name
and self.tasks == other.tasks
and self.collections == other.collections
)
return False

def __bool__(self):
def __bool__(self) -> bool:
return bool(self.task_names)

@classmethod
def from_module(
cls,
module,
name=None,
config=None,
loaded_from=None,
auto_dash_names=None,
):
module: ModuleType,
name: Optional[str] = None,
config: Optional[Dict[str, Any]] = None,
loaded_from: Optional[str] = None,
auto_dash_names: Optional[bool] = None,
) -> "Collection":
"""
Return a new `.Collection` created from ``module``.

Expand Down Expand Up @@ -198,7 +196,7 @@ def from_module(
"""
module_name = module.__name__.split(".")[-1]

def instantiate(obj_name=None):
def instantiate(obj_name: Optional[str] = None) -> "Collection":
# Explicitly given name wins over root ns name (if applicable),
# which wins over actual module name.
args = [name or obj_name or module_name]
Expand All @@ -218,7 +216,9 @@ def instantiate(obj_name=None):
ret = instantiate(obj_name=obj.name)
ret.tasks = ret._transform_lexicon(obj.tasks)
ret.collections = ret._transform_lexicon(obj.collections)
ret.default = ret.transform(obj.default)
ret.default = (
ret.transform(obj.default) if obj.default else None
Copy link
Member

Choose a reason for hiding this comment

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

Doing this without further altering Collection.transform() seems silly offhand; feels like we want to do one of:

  1. acknowledge that transform may sometimes be handed None and will do the right thing with it (becomes the identity function, basically, and hands None back to you), and change its signature to read eg: def transform(self, name: Union[str, None]) -> Union[str, None]:.
    • In this case, the line I am writing this under, would go back to how it was before.
  2. expect that all callers of transform will be running type-checking, will never be handing it anything but str, and nuke the escape-hatch at the top of the method
    • In this case, this line stays as proposed, though it might behoove us to search harder for other lines that are as ambiguous as the original here (something I'm still trying to wrap my head around 😇 )

My "hasn't done much typing but has done lots of maintaining" gut says option 1 is the right move here: less behavior/code change (as in a tiny amount would still be more than zero!) and in line with original intent of first implementation ("this method will be liberal in what it accepts").

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's internal code, not part of the API, I'd probably go for the second option if it results in cleaner code.

If you go for the first option though, you'll want to type as:

_NameT = TypeVar("_NameT", str, None)
def transform(self, name: _NameT) -> _NameT:

Which means the return type is the same as the parameter.

i.e. reveal_type(transform("foo")) would give you str. If you used unions, it'd give you str | None.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO transform doesn't actually do anything when None and passing None to every other caller would still require the checks. I'd rather throw an exception TBH.

)
# Explicitly given config wins over root ns config
obj_config = copy_dict(obj._configuration)
if config:
Expand All @@ -235,7 +235,13 @@ def instantiate(obj_name=None):
collection.configure(config)
return collection

def add_task(self, task, name=None, aliases=None, default=None):
def add_task(
self,
task: "Task",
name: Optional[str] = None,
aliases: Optional[Tuple[str, ...]] = None,
default: Optional[bool] = None,
) -> None:
"""
Add `.Task` ``task`` to this collection.

Expand All @@ -258,8 +264,9 @@ def add_task(self, task, name=None, aliases=None, default=None):
if name is None:
if task.name:
name = task.name
# XXX https://github.com/python/mypy/issues/1424
elif hasattr(task.body, "func_name"):
name = task.body.func_name
name = task.body.func_name # type: ignore
elif hasattr(task.body, "__name__"):
name = task.__name__
else:
Expand All @@ -275,7 +282,12 @@ def add_task(self, task, name=None, aliases=None, default=None):
self._check_default_collision(name)
self.default = name

def add_collection(self, coll, name=None, default=None):
def add_collection(
self,
coll: "Collection",
name: Optional[str] = None,
default: Optional[bool] = None,
) -> None:
"""
Add `.Collection` ``coll`` as a sub-collection of this one.

Expand All @@ -294,7 +306,7 @@ def add_collection(self, coll, name=None, default=None):
Added the ``default`` parameter.
"""
# Handle module-as-collection
if isinstance(coll, types.ModuleType):
if isinstance(coll, ModuleType):
coll = Collection.from_module(coll)
# Ensure we have a name, or die trying
name = name or coll.name
Expand All @@ -311,12 +323,12 @@ def add_collection(self, coll, name=None, default=None):
self._check_default_collision(name)
self.default = name

def _check_default_collision(self, name):
def _check_default_collision(self, name: str) -> None:
if self.default:
msg = "'{}' cannot be the default because '{}' already is!"
raise ValueError(msg.format(name, self.default))

def _split_path(self, path):
def _split_path(self, path: str) -> Tuple[str, str]:
"""
Obtain first collection + remainder, of a task path.

Expand All @@ -331,7 +343,7 @@ def _split_path(self, path):
rest = ".".join(parts)
return coll, rest

def subcollection_from_path(self, path):
def subcollection_from_path(self, path: str) -> "Collection":
"""
Given a ``path`` to a subcollection, return that subcollection.

Expand All @@ -343,7 +355,7 @@ def subcollection_from_path(self, path):
collection = collection.collections[parts.pop(0)]
return collection

def __getitem__(self, name=None):
def __getitem__(self, name: Optional[str] = None) -> Any:
"""
Returns task named ``name``. Honors aliases and subcollections.

Expand All @@ -359,11 +371,15 @@ def __getitem__(self, name=None):
"""
return self.task_with_config(name)[0]

def _task_with_merged_config(self, coll, rest, ours):
def _task_with_merged_config(
self, coll: str, rest: str, ours: Dict[str, Any]
) -> Tuple[str, Dict[str, Any]]:
task, config = self.collections[coll].task_with_config(rest)
return task, dict(config, **ours)

def task_with_config(self, name):
def task_with_config(
self, name: Optional[str]
) -> Tuple[str, Dict[str, Any]]:
"""
Return task named ``name`` plus its configuration dict.

Expand Down Expand Up @@ -397,14 +413,16 @@ def task_with_config(self, name):
# Regular task lookup
return self.tasks[name], ours

def __contains__(self, name):
def __contains__(self, name: str) -> bool:
try:
self[name]
return True
except KeyError:
return False

def to_contexts(self, ignore_unknown_help=None):
def to_contexts(
self, ignore_unknown_help: Optional[bool] = None
) -> List[ParserContext]:
"""
Returns all contained tasks and subtasks as a list of parser contexts.

Expand All @@ -430,12 +448,12 @@ def to_contexts(self, ignore_unknown_help=None):
)
return result

def subtask_name(self, collection_name, task_name):
def subtask_name(self, collection_name: str, task_name: str) -> str:
return ".".join(
[self.transform(collection_name), self.transform(task_name)]
)

def transform(self, name):
def transform(self, name: str) -> str:
"""
Transform ``name`` with the configured auto-dashes behavior.

Expand Down Expand Up @@ -474,25 +492,25 @@ def transform(self, name):
replaced.append(char)
return "".join(replaced)

def _transform_lexicon(self, old):
def _transform_lexicon(self, old: Lexicon) -> Lexicon:
"""
Take a Lexicon and apply `.transform` to its keys and aliases.

:returns: A new Lexicon.
"""
new_ = Lexicon()
new = Lexicon()
# Lexicons exhibit only their real keys in most places, so this will
# only grab those, not aliases.
for key, value in old.items():
# Deepcopy the value so we're not just copying a reference
new_[self.transform(key)] = copy.deepcopy(value)
new[self.transform(key)] = copy.deepcopy(value)
# Also copy all aliases, which are string-to-string key mappings
for key, value in old.aliases.items():
new_.alias(from_=self.transform(key), to=self.transform(value))
return new_
new.alias(from_=self.transform(key), to=self.transform(value))
return new

@property
def task_names(self):
def task_names(self) -> Dict[str, List[str]]:
"""
Return all task identifiers for this collection as a one-level dict.

Expand Down Expand Up @@ -523,7 +541,7 @@ def task_names(self):
ret[self.subtask_name(coll_name, task_name)] = aliases
return ret

def configuration(self, taskpath=None):
def configuration(self, taskpath: Optional[str] = None) -> Dict[str, Any]:
"""
Obtain merged configuration values from collection & children.

Expand All @@ -541,7 +559,7 @@ def configuration(self, taskpath=None):
return copy_dict(self._configuration)
return self.task_with_config(taskpath)[1]

def configure(self, options):
def configure(self, options: Dict[str, Any]) -> None:
"""
(Recursively) merge ``options`` into the current `.configuration`.

Expand All @@ -560,7 +578,7 @@ def configure(self, options):
"""
merge_dicts(self._configuration, options)

def serialized(self):
def serialized(self) -> Dict[str, Any]:
"""
Return an appropriate-for-serialization version of this object.

Expand Down
Loading