-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Make Teleprompt.compile more typing friendly.
#9013
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
base: main
Are you sure you want to change the base?
Conversation
…racks the type exact type of the student module passed in
|
Thanks @TyTodd! Is it hard to have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request introduces improved type safety to the DSPy teleprompt system by adding generic type variables. The main changes include introducing TypeVar("M", bound=Module) to preserve the specific type of the student module through the compile method, replacing previous Module or Any return types.
- Added
TypeVarimports and definedM = TypeVar("M", bound=Module)in multiple teleprompt files - Updated
compilemethod signatures to use generic typeMinstead ofModuleorAnyfor better type preservation - Applied consistent code formatting (line breaks, spacing) across multiple files
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| dspy/teleprompt/teleprompt.py | Added TypeVar M and updated base Teleprompter.compile signature to use generic type |
| dspy/teleprompt/simba.py | Added TypeVar M, updated SIMBA.compile signature, and reformatted docstrings |
| dspy/teleprompt/mipro_optimizer_v2.py | Added TypeVar M, updated MIPROv2.compile signature, and reformatted long lines |
| dspy/teleprompt/grpo.py | Added TypeVar M, updated GRPO.compile signature, and reformatted long lines/assertions |
| dspy/teleprompt/bootstrap_finetune.py | Added TypeVar M and updated FinetuneTeleprompter.compile signature |
| dspy/teleprompt/bettertogether.py | Added TypeVar M, updated BetterTogether.compile signature, and reformatted code |
Comments suppressed due to low confidence (3)
dspy/teleprompt/bettertogether.py:7
- The import
from dspy.primitives.module import Moduleon line 7 is redundant since line 17 already importsfrom dspy.primitives import Module. One of these imports should be removed to avoid duplication.
from dspy.primitives.module import Module
dspy/teleprompt/bettertogether.py:90
- The return type of
_run_strategiesshould beMinstead ofModuleto maintain consistency with the generic type used in thecompilemethod signature. This would ensure the type is preserved through internal method calls.
def _run_strategies(self, parsed_strategy, student, trainset, valset_ratio) -> Module:
dspy/teleprompt/bettertogether.py:128
- The return type of
_compile_prompt_optimizershould use the generic typeMinstead ofModulefor consistency with the type safety improvements introduced in this PR.
def _compile_prompt_optimizer(self, student, trainset, valset_ratio) -> Module:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def compile( | ||
| self, | ||
| student: M, | ||
| *, | ||
| trainset: list[Example], | ||
| teacher: Module | None = None, | ||
| valset: list[Example] | None = None, | ||
| **kwargs, | ||
| ) -> M: |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overridden method signature does not match call, where it is passed too many arguments. Overriding method method BootstrapFinetune.compile matches the call.
Overridden method signature does not match call, where it is passed too many arguments. Overriding method method BootstrapFinetune.compile matches the call.
Overridden method signature does not match call, where it is passed an argument named 'trainset'. Overriding method method BootstrapFinetune.compile matches the call.
Overridden method signature does not match call, where it is passed an argument named 'trainset'. Overriding method method BootstrapFinetune.compile matches the call.
Overridden method signature does not match call, where it is passed an argument named 'teacher'. Overriding method method BootstrapFinetune.compile matches the call.
| def compile( | ||
| self, | ||
| student: Module, | ||
| student: M, | ||
| trainset: list[Example], | ||
| strategy: str = "p -> w -> p", | ||
| valset_ratio = 0.1, | ||
| ) -> Module: | ||
| valset_ratio=0.1, | ||
| ) -> M: |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method requires at least 3 positional arguments, whereas overridden Teleprompter.compile requires 2.
| def compile(self, student: M, trainset: list[Example], teacher: Module | list[Module] | None = None) -> M: | ||
| # TODO: Print statements can be converted to logger.info if we ensure | ||
| # that the default DSPy logger logs info level messages in notebook | ||
| # environments. |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method requires at least 3 positional arguments, whereas overridden Teleprompter.compile requires 2.
| def compile(self, student: M, trainset: list[Example], teacher: Module | list[Module] | None = None) -> M: | |
| # TODO: Print statements can be converted to logger.info if we ensure | |
| # that the default DSPy logger logs info level messages in notebook | |
| # environments. | |
| def compile(self, student: M, trainset: list[Example], **kwargs) -> M: | |
| # TODO: Print statements can be converted to logger.info if we ensure | |
| # that the default DSPy logger logs info level messages in notebook | |
| # environments. | |
| teacher = kwargs.get('teacher', None) |
| def compile( | ||
| self, | ||
| student: Module, | ||
| student: M, | ||
| trainset: list[Example], | ||
| teacher: Module | list[Module] | None = None, | ||
| valset: list[Example] | None = None, | ||
| **kwargs, | ||
| ) -> Module: | ||
| logger.info("Starting the GRPO compilation process... The LM(s) for the student program will be updated in place at the end of the training.") | ||
| ) -> M: |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method requires at least 3 positional arguments, whereas overridden Teleprompter.compile requires 2.
Thanks for your response! It's not really hard. It's more of a taste thing I guess. |
| launched_flag = False | ||
|
|
||
| for ind, step_code in enumerate(parsed_strategy): | ||
| current_strategy = self.STRAT_SEP.join(parsed_strategy[:ind + 1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not include unrelated changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must be my formatter. Let me see if I can undo the other formatting changes.
|
I see, using base class in type hint is very common but I also understand where you come from. @okhat @chenmoneygithub what do you think about using generic in the type hint of .compile? |
I replaced
student: ModuleinTeleprompt.compile's signature withstudent: M, a TypeVar that tracks the type and returns that same typeI also applied this signature to all other compilers in the SDK.
It doesn't change the functionality of anything at all. My main reason for adding this is I (and others) often add custom functionality to
dspy.Modules and its just convenient to have the type checker still recognize those functions and attributes on the optimized program after running compile.For example, in this script the type checker will not highlight
weather_functionbecause the custom function is forgotten when the new program is returned fromcompile. With this new commit that is no longer the case.