Skip to content

support str|None , mapped_column, AnyURL#1

Merged
mbsantiago merged 8 commits intombsantiago:mainfrom
honglei:main
Aug 14, 2023
Merged

support str|None , mapped_column, AnyURL#1
mbsantiago merged 8 commits intombsantiago:mainfrom
honglei:main

Conversation

@honglei
Copy link

@honglei honglei commented Aug 12, 2023

@mbsantiago
Copy link
Owner

Hi @honglei! Thanks for the PR. I've run the checks and they are failing. It seems to be some typing issues. I'm a bit busy atm so can't work this week, but I could help with some checks the next one.

Copy link
Owner

@mbsantiago mbsantiago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for the PR! Please check this typing errors

sqlmodel/main.py Outdated
from typing import (
AbstractSet,
Any,
Annotated,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annotated was introduced in Python 3.9. Need to check python version and import it from typing_extensions if necessary

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, use typing_extensions to import Annotated.

sqlmodel/main.py Outdated
return AutoString(length=meta.max_length)

if get_origin(type_) is Annotated:
type2 = type_.__args__[0]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mypy does not like this line:

Item "type" of "Optional[type]" has no attribute "args"

Maybe use https://docs.python.org/3/library/typing.html#typing.get_args

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! typing.get_args is much better.

sqlmodel/main.py Outdated
if get_origin(type_) is Annotated:
type2 = type_.__args__[0]
if type2 is pydantic.AnyUrl:
meta = type_.__metadata__[0]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Item "None" of "Optional[type]" has no attribute "metadata"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will use typing.get_args

@honglei honglei requested a review from mbsantiago August 14, 2023 13:05
Copy link
Owner

@mbsantiago mbsantiago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small fixes

@mbsantiago
Copy link
Owner

@honglei Almost all typing errors are fixed. But there are some formatting errors now :/. Please consult the workflow logs to see the issues.

@honglei
Copy link
Author

honglei commented Aug 14, 2023

@honglei Almost all typing errors are fixed. But there are some formatting errors now :/. Please consult the workflow logs to see the issues.

I use ruff checked in Python3.7/debain10, all passed except code length.

@mbsantiago mbsantiago merged commit f67b414 into mbsantiago:main Aug 14, 2023
mbsantiago pushed a commit that referenced this pull request Aug 25, 2023
fix .model_copy(...) Pydantic V2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants