-
Notifications
You must be signed in to change notification settings - Fork 259
fix: incomplete type hint for Regex pattern #1209
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
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.
LGTM
Co-authored-by: ᑕᗩᑭTᗩIᑎᗰᗩᖇᐯᗴᒪ <91954476+CAPITAINMARVEL@users.noreply.github.com>
Co-authored-by: Virtual1ty <staticxterm@users.noreply.github.com>
Co-authored-by: ᑕᗩᑭTᗩIᑎᗰᗩᖇᐯᗴᒪ <91954476+CAPITAINMARVEL@users.noreply.github.com>
another typo from me
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.
hey, I can't understand the change, should we change it to:
...
pattern: Union[AnyStr, re.Pattern[AnyStr]],
options: Optional[str] = None,
...
No because pymongo will raise an error ($regex has to be a string) |
fixed my typo and mypy
for more information, see https://pre-commit.ci
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.
lgtm
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.
Okay, it took us a while, but we finally got there. Looks good to me now.
Beanie's
Regex
Class can usere.Pattern[str]
aspattern
. In the background,bson.regex.Regex
's.from_native
method support it. So I add it to type hint.In exist tests, we also can see that:
beanie/tests/odm/test_encoder.py
Lines 49 to 55 in 53123ce