Skip to content

Commit

Permalink
Revisit mypy setup (#3485)
Browse files Browse the repository at this point in the history
* Move mypy setup from pre-commit
* Fix no implicit optionals
* Fix Optional[...] must have exactly one type argument
* Fix mypy type incompatibility errors
* Address comment about order of type | None

---------

Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
  • Loading branch information
merelcht authored Jan 11, 2024
1 parent 8ad96d5 commit 5e80b79
Show file tree
Hide file tree
Showing 27 changed files with 186 additions and 203 deletions.
44 changes: 0 additions & 44 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,6 @@ repos:
- id: requirements-txt-fixer # Sorts entries in requirements.txt
exclude: "^kedro/templates/|^features/steps/test_starter/"

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.961
hooks:
- id: mypy
args: [--allow-redefinition, --ignore-missing-imports]
exclude: |
(?x)(
^kedro/templates/|
^docs/|
^features/steps/test_starter/
)
additional_dependencies:
- types-cachetools
- types-filelock
- types-PyYAML
- types-redis
- types-requests
- types-setuptools
- types-toml
- attrs

- repo: https://github.com/asottile/blacken-docs
rev: v1.12.1
hooks:
Expand All @@ -69,29 +48,8 @@ repos:
pass_filenames: false
entry: lint-imports

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.961
hooks:
- id: mypy
args: [--allow-redefinition, --ignore-missing-imports]
exclude: |
(?x)(
^kedro/templates/|
^docs/|
^features/steps/test_starter/
)
additional_dependencies:
- types-cachetools
- types-filelock
- types-PyYAML
- types-redis
- types-requests
- types-setuptools
- types-toml
- attrs
- repo: local
hooks:
# Slow lintint
- id: secret_scan
name: "Secret scan"
language: system
Expand All @@ -104,5 +62,3 @@ repos:
types: [file, python]
exclude: ^kedro/templates/|^tests/|^features/steps/test_starter
entry: bandit -ll

# Manual only
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ clean:

lint:
pre-commit run -a --hook-stage manual $(hook)
mypy kedro
test:
pytest --numprocesses 4 --dist loadfile

Expand Down
1 change: 1 addition & 0 deletions docs/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@
"D.get(k,d), also set D[k]=d if k not in D",
"D[k] if k in D, else d. d defaults to None.",
"None. Update D from mapping/iterable E and F.",
"Patterns",
),
"py:data": (
"typing.Any",
Expand Down
2 changes: 1 addition & 1 deletion docs/source/hooks/examples.md
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ class NodeInputReplacementHook:
@hook_impl
def before_node_run(
self, node: Node, catalog: DataCatalog
) -> Optional[Dict[str, Any]]:
) -> dict[str, Any] | None:
"""Replace `first_input` for `my_node`"""
if node.name == "my_node":
# return the string filepath to the `first_input` dataset
Expand Down
4 changes: 2 additions & 2 deletions kedro/config/abstract_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ class AbstractConfigLoader(UserDict):
def __init__(
self,
conf_source: str,
env: str = None,
runtime_params: dict[str, Any] = None,
env: str | None = None,
runtime_params: dict[str, Any] | None = None,
**kwargs,
):
super().__init__()
Expand Down
32 changes: 17 additions & 15 deletions kedro/config/omegaconf_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
import io
import logging
import mimetypes
import typing
from pathlib import Path
from typing import Any, Callable, Iterable

import fsspec
from omegaconf import OmegaConf
from omegaconf import DictConfig, OmegaConf
from omegaconf.errors import InterpolationResolutionError, UnsupportedInterpolationType
from omegaconf.resolvers import oc
from yaml.parser import ParserError
Expand Down Expand Up @@ -76,14 +77,14 @@ class OmegaConfigLoader(AbstractConfigLoader):
def __init__( # noqa: PLR0913
self,
conf_source: str,
env: str = None,
runtime_params: dict[str, Any] = None,
env: str | None = None,
runtime_params: dict[str, Any] | None = None,
*,
config_patterns: dict[str, list[str]] = None,
base_env: str = None,
default_run_env: str = None,
custom_resolvers: dict[str, Callable] = None,
merge_strategy: dict[str, str] = None,
config_patterns: dict[str, list[str]] | None = None,
base_env: str | None = None,
default_run_env: str | None = None,
custom_resolvers: dict[str, Callable] | None = None,
merge_strategy: dict[str, str] | None = None,
):
"""Instantiates a ``OmegaConfigLoader``.
Expand Down Expand Up @@ -251,6 +252,7 @@ def __repr__(self): # pragma: no cover
f"config_patterns={self.config_patterns})"
)

@typing.no_type_check
def load_and_merge_dir_config( # noqa: PLR0913
self,
conf_path: str,
Expand Down Expand Up @@ -431,7 +433,7 @@ def _check_duplicates(seen_files_to_keys: dict[Path, set[Any]]):
raise ValueError(f"{dup_str}")

@staticmethod
def _resolve_environment_variables(config: dict[str, Any]) -> None:
def _resolve_environment_variables(config: DictConfig) -> None:
"""Use the ``oc.env`` resolver to read environment variables and replace
them in-place, clearing the resolver after the operation is complete if
it was not registered beforehand.
Expand Down Expand Up @@ -466,16 +468,16 @@ def _soft_merge(config, env_config):
# Soft merge the two env dirs. The chosen env will override base if keys clash.
return OmegaConf.to_container(OmegaConf.merge(config, env_config))

def _is_hidden(self, path: str):
def _is_hidden(self, path_str: str):
"""Check if path contains any hidden directory or is a hidden file"""
path = Path(path)
path = Path(path_str)
conf_path = Path(self.conf_source).resolve().as_posix()
if self._protocol == "file":
path = path.resolve()
path = path.as_posix()
if path.startswith(conf_path):
path = path.replace(conf_path, "")
parts = path.split(self._fs.sep) # filesystem specific separator
posix_path = path.as_posix()
if posix_path.startswith(conf_path):
posix_path = posix_path.replace(conf_path, "")
parts = posix_path.split(self._fs.sep) # filesystem specific separator
HIDDEN = "."
# Check if any component (folder or file) starts with a dot (.)
return any(part.startswith(HIDDEN) for part in parts)
32 changes: 17 additions & 15 deletions kedro/framework/cli/micropkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,10 @@ def pull_package( # noqa: PLR0913
def _pull_package( # noqa: PLR0913
package_path: str,
metadata: ProjectMetadata,
env: str = None,
alias: str = None,
destination: str = None,
fs_args: str = None,
env: str | None = None,
alias: str | None = None,
destination: str | None = None,
fs_args: str | None = None,
):
with tempfile.TemporaryDirectory() as temp_dir:
temp_dir_path = Path(temp_dir).resolve()
Expand Down Expand Up @@ -473,7 +473,7 @@ def _refactor_code_for_unpacking( # noqa: PLR0913
"""

def _move_package_with_conflicting_name(
target: Path, original_name: str, desired_name: str = None
target: Path, original_name: str, desired_name: str | None = None
) -> Path:
_rename_package(project, original_name, "tmp_name")
full_path = _create_nested_package(project, target)
Expand Down Expand Up @@ -524,9 +524,9 @@ def _install_files( # noqa: PLR0913, too-many-locals
project_metadata: ProjectMetadata,
package_name: str,
source_path: Path,
env: str = None,
alias: str = None,
destination: str = None,
env: str | None = None,
alias: str | None = None,
destination: str | None = None,
):
env = env or "base"

Expand Down Expand Up @@ -605,9 +605,9 @@ def _get_default_version(metadata: ProjectMetadata, micropkg_module_path: str) -
def _package_micropkg(
micropkg_module_path: str,
metadata: ProjectMetadata,
alias: str = None,
destination: str = None,
env: str = None,
alias: str | None = None,
destination: str | None = None,
env: str | None = None,
) -> Path:
micropkg_name = micropkg_module_path.split(".")[-1]
package_dir = metadata.source_dir / metadata.package_name
Expand Down Expand Up @@ -635,12 +635,14 @@ def _package_micropkg(
# Check that micropkg directory exists and not empty
_validate_dir(package_source)

destination = Path(destination) if destination else metadata.project_path / "dist"
package_destination = (
Path(destination) if destination else metadata.project_path / "dist"
)
version = _get_default_version(metadata, micropkg_module_path)

_generate_sdist_file(
micropkg_name=micropkg_name,
destination=destination.resolve(),
destination=package_destination.resolve(),
source_paths=source_paths,
version=version,
metadata=metadata,
Expand All @@ -650,7 +652,7 @@ def _package_micropkg(
_clean_pycache(package_dir)
_clean_pycache(metadata.project_path)

return destination
return package_destination


def _validate_dir(path: Path) -> None:
Expand Down Expand Up @@ -826,7 +828,7 @@ def _generate_sdist_file( # noqa: PLR0913,too-many-locals
source_paths: tuple[Path, Path, list[tuple[Path, str]]],
version: str,
metadata: ProjectMetadata,
alias: str = None,
alias: str | None = None,
) -> None:
package_name = alias or micropkg_name
package_source, tests_source, conf_source = source_paths
Expand Down
10 changes: 5 additions & 5 deletions kedro/framework/cli/starters.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class KedroStarterSpec: # noqa: too-few-public-methods
for starter_spec in _OFFICIAL_STARTER_SPECS:
starter_spec.origin = "kedro"

_OFFICIAL_STARTER_SPECS = {spec.alias: spec for spec in _OFFICIAL_STARTER_SPECS}
_OFFICIAL_STARTER_SPECS_DICT = {spec.alias: spec for spec in _OFFICIAL_STARTER_SPECS}

TOOLS_SHORTNAME_TO_NUMBER = {
"lint": "1",
Expand Down Expand Up @@ -308,7 +308,7 @@ def new( # noqa: PLR0913


@starter.command("list")
def list_starters():
def list_starters() -> None:
"""List all official project starters available."""
starters_dict = _get_starters_dict()

Expand Down Expand Up @@ -360,7 +360,7 @@ def _get_cookiecutter_dir(
f" Specified tag {checkout}. The following tags are available: "
+ ", ".join(_get_available_tags(template_path))
)
official_starters = sorted(_OFFICIAL_STARTER_SPECS)
official_starters = sorted(_OFFICIAL_STARTER_SPECS_DICT)
raise KedroCliError(
f"{error_message}. The aliases for the official Kedro starters are: \n"
f"{yaml.safe_dump(official_starters, sort_keys=False)}"
Expand Down Expand Up @@ -438,7 +438,7 @@ def _get_starters_dict() -> dict[str, KedroStarterSpec]:
),
}
"""
starter_specs = _OFFICIAL_STARTER_SPECS
starter_specs = _OFFICIAL_STARTER_SPECS_DICT

for starter_entry_point in _get_entry_points(name="starters"):
origin = starter_entry_point.module.split(".")[0]
Expand Down Expand Up @@ -777,7 +777,7 @@ def _validate_selection(tools: list[str]):
sys.exit(1)


def _parse_tools_input(tools_str: None | str):
def _parse_tools_input(tools_str: str | None):
"""Parse the tools input string.
Args:
Expand Down
4 changes: 2 additions & 2 deletions kedro/framework/cli/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ def _merge_same_name_collections(groups: Sequence[click.MultiCommand]):
named_groups: defaultdict[str, list[click.MultiCommand]] = defaultdict(list)
helps: defaultdict[str, list] = defaultdict(list)
for group in groups:
named_groups[group.name].append(group)
named_groups[group.name].append(group) # type: ignore
if group.help:
helps[group.name].append(group.help)
helps[group.name].append(group.help) # type: ignore

return [
click.CommandCollection(
Expand Down
4 changes: 2 additions & 2 deletions kedro/framework/context/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ def params(self) -> dict[str, Any]:

def _get_catalog(
self,
save_version: str = None,
load_versions: dict[str, str] = None,
save_version: str | None = None,
load_versions: dict[str, str] | None = None,
) -> DataCatalog:
"""A hook for changing the creation of a DataCatalog instance.
Expand Down
Loading

0 comments on commit 5e80b79

Please sign in to comment.