-
Notifications
You must be signed in to change notification settings - Fork 633
Use Pydantic for type annotations #407
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
Merged
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
This was referenced Feb 8, 2022
evilstreak
added a commit
that referenced
this pull request
Feb 14, 2022
These are on the PR: #407 They should be copied to release notes once merged, and then this branch can be deleted.
This was referenced Feb 15, 2022
Signed-off-by: Ben Firshman <[email protected]>
Signed-off-by: Ben Firshman <[email protected]>
Signed-off-by: Ben Firshman <[email protected]>
Signed-off-by: Ben Firshman <[email protected]>
Signed-off-by: Ben Firshman <[email protected]>
Signed-off-by: Zeke Sikelianos <[email protected]>
Signed-off-by: Zeke Sikelianos <[email protected]>
..even when the user-defined predict function returns a simple type like a string. Co-Authored-By: Ben Firshman <[email protected]> Signed-off-by: Zeke Sikelianos <[email protected]>
Signed-off-by: Ben Firshman <[email protected]>
Signed-off-by: Zeke Sikelianos <[email protected]>
As a convenience for `from pydantic import BaseModel` Signed-off-by: Ben Firshman <[email protected]>
Signed-off-by: Zeke Sikelianos <[email protected]>
Signed-off-by: Zeke Sikelianos <[email protected]>
Signed-off-by: Zeke Sikelianos <[email protected]>
These all don't seem relevant (and in some cases undocumented?). I have also removed support for `kwargs`, which just shows up as extra fields in the schema. What a lovely foot gun that is. They aren't even put behind `additionalProperties`, or something like that. Adding support for things is much easier to take it away, so it seems to be conservative here. Signed-off-by: Ben Firshman <[email protected]>
Installing 1.21.1 failed locally because the binary is only available for older versions of Python. Although the latest version of numpy is 1.22.2, the latest version that includes support for Python 3.7 is 1.21.5. Signed-off-by: Dominic Baggott <[email protected]>
The new Cog internals return a 422 instead of a 400 when the input is invalid. Rather than dumping that out as a cryptic "returned a 422" message, pull out the individual validation issues from the response and present them to the user. Adding the hint about how to provide inputs on the command line will hopefully help first time users who are running a bare `cog predict`. Signed-off-by: Dominic Baggott <[email protected]>
If an output type isn't specified, Pydantic expects the model to return nothing. When the model returns an output and it's compared to `None`, an error is logged and a 500 error is returned. This change explicitly raises an error when the output type is undefined and raises an error message, rather than defaulting to `None`. The error message provides explanation of how to fix the issue more clearly than the original behaviour. Signed-off-by: Dominic Baggott <[email protected]>
The source of this information is the annotation, but the thing we're actually using this for is the type of the input. Name it `InputType` to make that clearer, and for consistency with the variable `OutputType` used for type checking outputs. Co-authored-by: Preethi Vaidyanathan <[email protected]> Signed-off-by: Dominic Baggott <[email protected]>
When presenting the list of allowed types to the user: - show builtins as just the class name (eg `str`) - prefix other classes with the module (eg `cog.Path`) Within Cog, `Path` and `File` are available in the module `cog.types`, but when Cog is imported into another project they're in the module `cog`. Special case Cog types to only show the `cog` module. Added a test to assert an error is raised when starting the server. Moved, renamed and tweaked the test for extraneous keys, because the server won't start any more with that predictor definition. Co-authored-by: Preethi Vaidyanathan <[email protected]> Signed-off-by: Dominic Baggott <[email protected]>
The message coming from Pydantic isn't quite what we'd write ourselves, but it's good enough for a model author to debug the issue. Signed-off-by: Dominic Baggott <[email protected]>
Signed-off-by: Ben Firshman <[email protected]>
Signed-off-by: Ben Firshman <[email protected]>
* document cog.Path() Signed-off-by: Zeke Sikelianos <[email protected]> * document cog.File() Signed-off-by: Zeke Sikelianos <[email protected]> * fix import Signed-off-by: Zeke Sikelianos <[email protected]> * whoops don't forget Input Signed-off-by: Zeke Sikelianos <[email protected]> * update docs for cog.File and cog.Path based on feedback from Benzo Signed-off-by: Zeke Sikelianos <[email protected]>
zeke
approved these changes
Mar 1, 2022
Member
zeke
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.
👏🏼 PY 👏🏼 DAN 👏🏼 TIC!
![]()
Member
Author
|
All rebased and merge-conflict-resolved. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is an implementation of #193 and #205 to use Pydantic as the type annotations for model predictors and FastAPI for serving the model via HTTP.
Models now look a bit like this:
This is a follow-up of #378, where the initial working version was made.
At some point, we want to lock down and define the HTTP API, but we are deferring that to a future piece of work. The Redis queue worker remains unchanged and supports the old API for Replicate to run these models.
The branch has now been moved to this repository so we can collaborate together by opening pull requests against it. We aren't putting this work in
mainbecause the documentation for the current version of Cog is there.Upgrading from Cog 0.0 to 0.1
[Note: This documentation should be copied to the release notes when we merge this.]
In Cog versions up to 0.0.20 you described inputs using annotations on your
predictmethod. For example:From Cog 0.1.0 onwards, we've started using Pydantic to define input and output types. Rather than describing inputs using annotations, you now describe them with type hinting. Here's how you'd define the same inputs now:
The parameters that
Input()takes are pretty similar to those@cog.input()used to take. Here are the differences:typeparameter; use a type hint instead.helpparameter has been renamed todescription.optionsparameter has been renamed tochoice.minoption has been replaced by two options:ge: greater than or equal to (direct replacement)gt: greater than (a new alternative)maxoption has been replaced by two options:le: less than or equal to (direct replacement)lt: less than (a new alternative)The other major difference is that you now need to define the output type of your model. That's the
-> Pathbit in the example above. That might be a simple type likestr,floatorbool. If you need to handle multiple outputs, check out the new documentation for complex output objects. If you only have a single output, but it can be of different types depending on the run, you can usetyping.Any:Todo
cog.Input(). Remove non-essential arguments tocog.Input()#408cog predictin a simple way until we lock down HTTP API. Better error message when providing invalid inputs #420Any, but they need to be explicit!) Display better error when output type is undefined #421@input()to Pydantic type annotations.push_error()in the redis worker should be called with an appropriate human-readable error.cog.Pathandcog.File#445Future
pathlib.Pathas an input type? Does it work? Should we throw an error? (This might happen if you import wrong package / collide imports) [It breaks! Strictly limit the allowed model input types #429 prevents it being used as an input type]. See also Update pydantic Cog to serialize pathlib Paths #444pathlib.Pathfrom a predict function? Does this work? Should it throw an error? Only allow particular output types #456Do we want to makeWith a shim, this will just work.cog predictwith an existing image backwards compatible? ('cos there are old models on Replicate)titlein OpenAPI schema #457(These should be migrated to issues.)Done!Refs
Closes #58
Closes #113
Closes #193
Closes #205
Closes #212
Closes #246
Closes #304
Closes #306
Closes #328