Skip to content

Conversation

@kitagry
Copy link
Contributor

@kitagry kitagry commented Dec 21, 2025

Description

This PR adds a type stub file (parameter.pyi) for Luigi's Parameter classes to provide better type hints and IDE support. Additionally, it applies the @dataclass_transform decorator to the Task class to enable dataclass-like type checking behavior.

Key changes:

  • Added comprehensive luigi/parameter.pyi stub file with type definitions for all Parameter classes
  • Applied @dataclass_transform decorator to Task class for better type checker support
  • Added typing-extensions>=4.12.2 as a dependency
  • Used Generic types, TypedDict, and Unpack for precise type annotations

Motivation and Context

This change improves the developer experience when using Luigi by providing:

  • Better type checking in IDEs and type checkers (mypy, pyright, etc.) through stub files
  • Improved autocomplete and IntelliSense support
  • Dataclass-like type checking for Task classes via @dataclass_transform
  • More explicit type information for parameter values without modifying runtime code
  • Better static analysis capabilities for Luigi tasks

The stub file approach allows us to provide rich type information without impacting runtime performance or modifying the existing implementation. The @dataclass_transform decorator helps type checkers understand that Task classes behave similarly to dataclasses when it comes to parameter handling.

This enhancement helps catch type-related bugs earlier in the development process and makes the codebase more maintainable.

Implementation Details

  • The .pyi file provides type signatures for all Parameter variants including specialized types like EnumParameter, ChoiceParameter, NumericalParameter, etc.
  • Generic type variables are used to preserve type information (e.g., EnumParameter[EnumParameterType], ChoiceParameter[ChoiceType])
  • TypedDict with Unpack provides typed keyword arguments for parameter constructors
  • The @dataclass_transform decorator with field_specifiers=(Parameter,) enables proper type checking for Task classes

Have you tested this? If so, how?

Yes, I have tested this implementation with multiple type checkers:

✅ Working Type Checkers

  • pyright: Full support - type annotations work correctly with the .pyi stub file
  • pyrefly: Full support - type annotations work correctly with the .pyi stub file

⚠️ Limited Support Type Checkers

Test Example

import luigi
from enum import Enum
from typing import Optional

class Severity(Enum):
    DEBUG = 'debug'
    INFO = 'info'

class Sample(luigi.Task):
    s: str = luigi.Parameter()
    i: int = luigi.IntParameter()
    oi: Optional[int] = luigi.OptionalIntParameter()
    e: Severity = luigi.EnumParameter(enum=Severity)

    def run(self):
        s_value: str = self.s
        i_value: int = self.i
        oi_value: Optional[int] = self.oi
        e_value: Severity = self.e

pyright and pyrefly result: ✅ 0 errors
mypy and ty result: ❌ Type errors (can be resolved with custom plugin or # type: ignore)

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.

1 question, 1 request

dlstadther
dlstadther previously approved these changes Dec 31, 2025
@dlstadther
Copy link
Collaborator

@RRap0so The codecov check is blocking me from merging this PR. Would you be able to help here?

@kitagry
Copy link
Contributor Author

kitagry commented Dec 31, 2025

I rebased onto the latest master because some parameter's variables are added in #3369 . And, I fixed stubtest in
68d40bf .

dlstadther
dlstadther previously approved these changes Jan 1, 2026
@kitagry
Copy link
Contributor Author

kitagry commented Jan 6, 2026

Hello. Is there any updates?

@dlstadther
Copy link
Collaborator

I don't have permissions to override or disable the one failing coverage test. That test has always been temperamental. It used to not be required to merge. But since it became required, a Spotify employee, such as @RRap0so , must assist.

RRap0so
RRap0so previously approved these changes Jan 6, 2026
@RRap0so
Copy link
Contributor

RRap0so commented Jan 6, 2026

@kitagry The codecov/changes check is failing because luigi/mypy.py has an indirect coverage drop (-0.74%).

The uncovered lines are in the except ValueError handler:

  except ValueError:
      # If we couldn't infer from __new__, return Any
      return AnyType(TypeOfAny.unannotated)

Could you add a test case in test/mypy_test.py that exercises this path? You'll need a Parameter subclass where "default" is not in ctx.callee_arg_names — for example, a Parameter instantiated without a default argument.

@dlstadther looks like something changed and I can't just easily bypass things. I'll have to take a look but meanwhile that's a way. We can also disable this check if we feel like it doesn't bring much value. Let me know if you would prefer that and ping me on the PR so I can approve.

@kitagry kitagry dismissed stale reviews from RRap0so and dlstadther via dc88071 January 6, 2026 23:42
@kitagry kitagry force-pushed the add-parameter-pyi branch 2 times, most recently from dc88071 to 97235bd Compare January 7, 2026 00:04
@kitagry kitagry force-pushed the add-parameter-pyi branch from 97235bd to 044a2cd Compare January 7, 2026 10:22
@kitagry kitagry force-pushed the add-parameter-pyi branch from 044a2cd to d42cae4 Compare January 7, 2026 11:36
@kitagry
Copy link
Contributor Author

kitagry commented Jan 7, 2026

@RRap0so Thank you for the comment! I've simplified the code in d42cae4 and the CI checks are now passing!

@RRap0so RRap0so merged commit 585b29b into spotify:master Jan 7, 2026
40 checks passed
@kitagry kitagry deleted the add-parameter-pyi branch January 7, 2026 13:10
@kitagry
Copy link
Contributor Author

kitagry commented Jan 7, 2026

Thank you! I'm looking forward to the next release😍

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.

3 participants