Repository navigation
💚 Fix linting in CI - #1340
💚 Fix linting in CI#1340tiangolo merged 11 commits into
Conversation
Sorry, something went wrong.
tiangolo
left a comment
There was a problem hiding this comment.
Thank you @svlandeg! 🚀
I tweaked it a bit mainly to keep autocompletion and inline errors for final users. Although this reveals some areas that probably deserve some tweaking in the future. 🤔
I added some inline comments to share my mental process.
Thanks! 🚀
Sorry, something went wrong.
| if isinstance(model, type): | ||
| use_model = model | ||
| else: | ||
| use_model = model.__class__ | ||
| return use_model.model_fields |
There was a problem hiding this comment.
I was checking the error from Pydantic and I saw that they will deprecate accessing the model_fields from the instance in Pydantic v3, they should be accessed from the class (which makes sense).
I updated this logic here to handle that future use case. In a subsequent PR we can refactor the internal usage of this function to only pass the class and not the instance.
Sorry, something went wrong.
| __sqlmodel_relationships__: Dict[str, RelationshipInfo] | ||
| model_config: SQLModelConfig | ||
| model_fields: Dict[str, FieldInfo] # type: ignore[assignment] | ||
| model_fields: ClassVar[Dict[str, FieldInfo]] |
There was a problem hiding this comment.
As the idea is that this should now be only in the class and not instances, I think it's good to type it right away. It won't affect at runtime if people are using it, but will start warning them in their linters, so they can start preparing for this change in Pydantic v3.
Sorry, something went wrong.
|
|
||
| @classmethod | ||
| def model_validate( | ||
| def model_validate( # type: ignore[override] |
There was a problem hiding this comment.
I changed this to not use kwargs and instead ignore the type here, that way people will keep having autocompletion for the params, and kwargs won't swallow extra / invald params.
Seeing this, I realize it was probably not the best idea to include the extra update here. 🤔
Also, it's probably not needed for most use cases, maybe it would make sense to remove those docs, explain how to extend data to validate an object using just dicts, then deprecate using it, and finally at some point dropping the update...
But anyway, for now, it probably works to just # type: ignore and handle it on our side so that final users get the best developer experience / editor support.
Sorry, something went wrong.
| context: Union[Dict[str, Any], None] = None, | ||
| by_alias: bool = False, | ||
| context: Union[Any, None] = None, | ||
| by_alias: Union[bool, None] = None, |
There was a problem hiding this comment.
I like to keep the signature as close to the latest Pydantic, and then do the extra work internally to support the older versions, but this way people can get used to the new syntax and/or use it if they can already upgrade to the latest version.
Sorry, something went wrong.
| include=include, | ||
| exclude=exclude, | ||
| by_alias=by_alias, | ||
| by_alias=by_alias or False, |
There was a problem hiding this comment.
With this trick, we avoid the type when by_alias is None. 😅
Sorry, something went wrong.
| serialize_as_any: bool = False, | ||
| ) -> Dict[str, Any]: | ||
| if PYDANTIC_MINOR_VERSION < (2, 11): | ||
| by_alias = by_alias or False |
There was a problem hiding this comment.
With this we can handle older Pydantic versions but keep the most recent signature.
Sorry, something went wrong.
…. v2.10 + The else block added to fix linting errors (4b5ad42) for fastapi#1340 was obliterating the 'context' and 'serialize_as_any' attributes for versions >= 2.7 and < 2.11. + Fix by initializing extra_kwargs and then setting on a per-version level
Recent PRs have been failing due to an issue with the linting step in the CI - I think caused by the recent release of Pydantic 2.11.
Overview of issues and proposed fixes:
I think it's fine to ignore this one?
The linting test is failing for Python 3.8 in different ways than for all other Python versions, probably because Python 3.8 support was removed for Pydantic 2.11. Accordingly, I've updated the test.yml to skip the linting test for Python 3.8. This doesn't feel like a clean solution either, and I guess the other option is to (also) drop support for Python 3.8.