Skip to content

feat: type check parameter classes#3406

Open
kitagry wants to merge 1 commit intospotify:masterfrom
kitagry:type-check-parameter
Open

feat: type check parameter classes#3406
kitagry wants to merge 1 commit intospotify:masterfrom
kitagry:type-check-parameter

Conversation

@kitagry
Copy link
Contributor

@kitagry kitagry commented Mar 8, 2026

Description

Enable type checking for luigi/parameter.py.

Motivation and Context

Have you tested this? If so, how?

@kitagry kitagry requested review from a team and dlstadther as code owners March 8, 2026 15:06
Copy link
Collaborator

@dlstadther dlstadther left a comment

Choose a reason for hiding this comment

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

There are quite a number of type-ignores on here. Are they included because we you gave up trying to solve them? Or because the design of the parameter system (and typing) doesn't have a solution?
There still more value having a subset of typechecks than no type checks.

@kitagry
Copy link
Contributor Author

kitagry commented Mar 15, 2026

These type: ignore comments are due to a fundamental limitation of mypy's support for Generic mixins with cooperative multiple inheritance.

OptionalParameterMixin uses super() calls whose MRO is only resolved at runtime. Mypy cannot statically determine the type of super() in this context, which causes misc/call-arg/arg-type errors. The same applies to the subclass definitions (OptionalIntParameter etc.) where mypy detects conflicting __get__ overloads from different base classes.

These are not "gave up" situations — they reflect the design of the existing mixin pattern, which Python's type system cannot fully express today. Resolving this properly would require reworking OptionalParameterMixin itself (e.g., replacing the mixin pattern with composition or a factory approach), which is out of scope for this PR. The suppressed errors are intentionally scoped to this mixin boundary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants