Fix issues flagged by mypy strict

[merelcht]

Description

Part of #2156 and continuation of #3485

I'm running mypy kedro --strict --allow-any-generics to find the issues and fix them. There's a lot of them, so I'll split the work up in different PRs. This being the first.

Development notes

• Running with --allow-any-generics, because disallowing it made mypy flag a lot of errors that weren't possible to fix https://mypy.readthedocs.io/en/stable/error_code_list2.html#check-that-type-arguments-exist-type-arg
• Made all # type: ignore specific to a error code
• Disabled error code ['misc'] because it was flagging stuff that couldn't be fixed like:

kedro/framework/cli/micropkg.py:307: error: Untyped decorator makes function "package_micropkg" untyped  [misc]

Developer Certificate of Origin

We need all contributions to comply with the Developer Certificate of Origin (DCO). All commits must be signed off by including a Signed-off-by line in the commit message. See our wiki for guidance.

If your PR is blocked due to unsigned commits, then you must follow the instructions under "Rebase the branch" on the GitHub Checks page for your PR. This will retroactively add the sign-off to all unsigned commits and allow the DCO check to pass.

Checklist

• Read the contributing guidelines
• Signed off each commit with a Developer Certificate of Origin (DCO)
• Opened this PR as a 'Draft Pull Request' if it is work-in-progress
• Updated the documentation to reflect the code changes
• Added a description of this change in the RELEASE.md file
• Added tests to cover my changes
• Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

[astrojuanlu]

Upon quick inspection I didn't find anything problematic but I'll defer to others to do a proper review

[DimedS]

Thank you, @merelcht ! A lot of work has been done, LGTM.

[noklam]

I found this a bit odd and run the same mypy check locally, I didn't find mypy complains the original signature.

Here we don't really expect None to be passed, I see that you have to add another guard clause to check if not cookiedcutter_context. If we apply this logic everywhere, we need to check for every single argument for None, thus I am curious what make this function a special case.

kedro/framework/cli/starters.py:393: error: Call to untyped function "_validate_selected_tools" in typed context [no-untyped-call]
kedro/framework/cli/starters.py:397: error: Call to untyped function "_validate_regex" in typed context [no-untyped-call]
kedro/framework/cli/starters.py:401: error: Call to untyped function "_validate_regex" in typed context [no-untyped-call]
kedro/framework/cli/starters.py:404: error: Returning Any from function declared to return "dict[str, Any]" [no-any-return]
kedro/framework/cli/starters.py:445: error: Call to untyped function "_safe_load_entry_point" in typed context [no-untyped-call]
kedro/framework/cli/starters.py:523: error: Unused "type: ignore" comment [unused-ignore]
kedro/framework/cli/starters.py:595: error: Returning Any from function declared to return "dict[str, str]" [no-any-return]
kedro/framework/cli/starters.py:635: error: Function is missing a return type annotation [no-untyped-def]
kedro/framework/cli/starters.py:643: error: Function is missing a return type annotation [no-untyped-def]
kedro/framework/cli/starters.py:705: error: Function is missing a return type annotation [no-untyped-def]
kedro/framework/cli/starters.py:742: error: Function is missing a return type annotation [no-untyped-def]

[merelcht]

Yeah this one took me a while to fix. The original error is:

kedro/framework/cli/starters.py:287: error: Argument "cookiecutter_context" to "_get_extra_context" has incompatible type "OrderedDict[Any, Any] | None"; expected "OrderedDict[Any, Any]"  [arg-type]

The problem is that mypy can't figure out that _fetch_config_from_user_prompts will never be called when cookiecutter_context is None. I don't know if there's a more elegant solution to fix this.

[AhdraMeraliQB]

The only fix I could see is changing the condition that calls _fetch_config_from_user_prompts to be if cookiecutter_context, which would work given we have a cookiecutter context if and only if we don't have a config path. However, this isn't representative of the actual logic and just exploits the iff condition - I don't think satisfying mypy is worth it. I'm happy to keep Ordered Dict | None in the type hint and add a line to the docstring below explaining that the cookiecutter context cannot be None (and hence a guardrail condition has been omitted).

Alternatively we could add a guardrail condition for consistency.

[merelcht]

I don't think satisfying mypy is worth it

So would you prefer to add an type: ignore here over the current solution?

[AhdraMeraliQB]

I should clarify, I don't think satisfying mypy is worth restructuring how we perform our condition checks in _get_extra_context . I'd prefer to leave the type hint as is with a note in the docstring that cookiecutter_context is never expected to be None / _fetch_config_from_user_prompts will only be called with a cookiecutter_context

[AhdraMeraliQB]

These are a lot of changes, thank you @merelcht, can't wait to see this go in!

Left some comments and questions, but nothing blocking! 🌟

[AhdraMeraliQB]

The only fix I could see is changing the condition that calls _fetch_config_from_user_prompts to be if cookiecutter_context, which would work given we have a cookiecutter context if and only if we don't have a config path. However, this isn't representative of the actual logic and just exploits the iff condition - I don't think satisfying mypy is worth it. I'm happy to keep Ordered Dict | None in the type hint and add a line to the docstring below explaining that the cookiecutter context cannot be None (and hence a guardrail condition has been omitted).

Alternatively we could add a guardrail condition for consistency.