Enhance types #93
Open
Enhance types #93
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Dear @mwildehahn , Thank you very much for the PR. I love types! Pointers:
|
|
Yup, sounds good.
…On Mon, Oct 6, 2025 at 12:31 PM Lakshya A Agrawal ***@***.***> wrote:
*LakshyAAAgrawal* left a comment (gepa-ai/gepa#93)
<#93 (comment)>
Dear @mwildehahn <https://github.com/mwildehahn> ,
Thank you very much for the PR. I love types!
Pointers:
1. Can you look at the failing ruff above?
2. Can you introduce type checking as part of the CI pipeline or
tests? I don't want strict type checking, as that can unnecessarily raise
the bar to make changes to the code, but I would like to have soft
type-checking, such that, the type checker ensures that all the type
annotations that are already present are satisfied, but those that aren't
are still OK. What do you think?
—
Reply to this email directly, view it on GitHub
<#93 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFMAUNRRCNMSMWEVUMXEBD3WK7SRAVCNFSM6AAAAACIEYUIECVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGNZTGU4DIOJQG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
pyrightconfig.json
Outdated
| "src/gepa/adapters/generic_rag_adapter", | ||
| "src/gepa/adapters/anymaths_adapter", | ||
| "src/gepa/adapters/terminal_bench_adapter", | ||
| "tests/test_rag_adapter/", |
There was a problem hiding this comment.
Shouldn't the entire tests directory be ignored?
tests/test_experiment_tracking.py
Outdated
| assert experiment is not None | ||
| assert experiment is not None | ||
| assert experiment is not None | ||
| assert experiment is not None |
There was a problem hiding this comment.
This seems to be an error
| scores.append(score) | ||
|
|
||
| if capture_traces: | ||
| if capture_traces and trajectories is not None: |
There was a problem hiding this comment.
This should be an assert instead.
src/gepa/api.py
Outdated
Comment on lines
33
to
57
| def _is_default_datainst(candidate: object) -> TypeGuard[DefaultDataInst]: | ||
| if not isinstance(candidate, dict): | ||
| return False | ||
| input_text = candidate.get("input") | ||
| additional_context = candidate.get("additional_context") | ||
| answer = candidate.get("answer") | ||
| if not isinstance(input_text, str) or not isinstance(answer, str): | ||
| return False | ||
| if not isinstance(additional_context, dict): | ||
| return False | ||
| return all(isinstance(key, str) and isinstance(value, str) for key, value in additional_context.items()) | ||
|
|
||
|
|
||
| def _is_default_dataset(dataset: Sequence[object]) -> TypeGuard[list[DefaultDataInst]]: | ||
| if not isinstance(dataset, list): | ||
| return False | ||
| return all(_is_default_datainst(item) for item in dataset) | ||
|
|
||
|
|
||
| def optimize( | ||
| seed_candidate: dict[str, str], | ||
| trainset: list[DataInst], | ||
| valset: list[DataInst] | None = None, | ||
| adapter: GEPAAdapter[DataInst, Trajectory, RolloutOutput] | None = None, | ||
| task_lm: str | Callable | None = None, | ||
| trainset: list[DataInstT], | ||
| valset: list[DataInstT] | None = None, | ||
| adapter: GEPAAdapter[DataInstT, TrajectoryT, RolloutOutputT] | None = None, | ||
| task_lm: str | TaskModelCallable | None = None, |
There was a problem hiding this comment.
Could you please clarify why these methods are created?
mwildehahn
force-pushed
the
mh/pyright
branch
from
2168f2e to
cf87e74
Compare
mwildehahn
commented
Oct 13, 2025
| RolloutOutputT = TypeVar("RolloutOutputT") | ||
|
|
||
|
|
||
| def _is_default_dataset(value: object) -> TypeGuard[list[DefaultDataInst]]: |
There was a problem hiding this comment.
@LakshyAAAgrawal I couldn't find a way around adding this type guard and the optimize + _optimize_impl. The issue is trying to maintain a fully generic optimize function but then also provide the default adapter if none is provided.
We have to do the type guard to validate the default dataset and then can init the default adapter and pass it.
I was trying to avoid changing the public API for this, lmk if there is something else you'd want to see here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LMK how you feel about this -- I want to start on #61 but I wanted to have a more solid foundation of types before adding to that so I installed pyright and then leveraged codex to add the minimal types necessary to get pyright passing. I left out the examples and there were also some issues with dspy since dspy depends on gepa there is a circular import.
I'm not trying to open a can of worms with dynamic vs strictly typed python but I find it much easier to maintain / extend when there are clear typing contracts of the library.