Skip to content
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

[contextgen] Honor parameterization in generator instance, expose parameters to serialize #1983

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sneakers-the-rat
Copy link
Collaborator

Builds on #1982

Noticed that the contextgen advertises parameters in its dataclass fields that it then ignores during serialization.

If those fields exist, they should do something!

set all args to default None and use instance attributes as defaults instead of method args as defaults - parameters can be set in the instance body, but overridden when calling serialize

@sneakers-the-rat
Copy link
Collaborator Author

looks like the failing tests are choices to make - the old behavior didn't honor the defaults set in the instance, so the tests confirm that. if we want that to be the case (ie. when you set a value in the generator instance it is actually used) then we need to change those tests probably. since this PR is unsolicited i'll wait until someone confirms this is the desired behavior before fixing

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

Successfully merging this pull request may close these issues.

None yet

1 participant