-
Notifications
You must be signed in to change notification settings - Fork 652
Support kwargs predictors #2345
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
Conversation
This is an experimental change to support **kwargs on predictors. You can annotate them as normal and the OpenAPI schema will reflect the type of **kwargs via additionalProperties.
Also to use `Union` rather than the pipe symbol which is not supported on older Python versions.
This unlocks our use case of using a cog model as a proxy without making the interface more complex by supporting partially defined inputs alongside the `**kwargs` parameter.
|
@philandstuff I've reworked this PR to be a bit more restrictive. We now only allow models to use the I think this is a good enough start for our use case. |
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 PR introduces experimental support for predictors that exclusively use **kwargs as input, enabling dynamic passthrough of input parameters for proxy models. Key changes include:
- Adding tests to verify kwargs input behavior.
- Implementing a new code branch in get_input_create_model_kwargs to support functions with only **kwargs.
- Updating the OpenAPI schema and adjusting create_model kwargs configuration.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| python/tests/server/test_http_input.py | Added test to validate dynamic kwargs inputs in predictions |
| python/tests/server/fixtures/input_kwargs.py | New predictor implementation accepting only **kwargs |
| python/cog/predictor.py | Updated input model generation logic to support exclusive **kwargs |
| .tool-versions | Updated Python version to 3.13.2 |
Comments suppressed due to low confidence (1)
python/tests/server/test_http_input.py:45
- [nitpick] The variable name 'input' shadows the built-in function. Consider renaming it to 'input_data' to improve clarity.
input = {"animal": "giraffe", "no": 5}
| if order != 0: | ||
| raise TypeError(f"Unsupported keyword parameter **{name}") | ||
|
|
||
| class ExtraKeywordInput(BaseInput): |
Copilot
AI
May 21, 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.
[nitpick] Consider adding a comment explaining the purpose of the 'ExtraKeywordInput' class to clarify its role in handling exclusive **kwargs inputs.
philandstuff
left a comment
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.
@aron thanks for the polish. LGTM
Co-authored-by: Philip Potter <[email protected]> Signed-off-by: Aron Carroll <[email protected]>
Co-authored-by: Philip Potter <[email protected]> Signed-off-by: Aron Carroll <[email protected]>
This change allows a predictor input to be defined exclusively as
**kwargs. This is an experimental change that allows a model to be used as a proxy where any inputs will be passed through directly without modification.This change restricts the use of
**kwargsto functions/methods that accept only that as an argument to avoid the complexity of having to support both dynamic and fixed inputs.The OpenAPI schema for such a model will be:
{ "properties": {}, "additionalProperties": true, "type": "object", "title": "Input" }