Skip to content
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

Fix compatibility issue with python <3.11 #45

Merged
merged 2 commits into from
Dec 3, 2023

Conversation

ManiMozaffar
Copy link
Contributor

@ManiMozaffar ManiMozaffar commented Dec 2, 2023

There's no tests yet?
didn't know where to place them

fixes #42.

@ManiMozaffar
Copy link
Contributor Author

ManiMozaffar commented Dec 2, 2023

a suggestion -> ui test would be definitely needed if you intend to write useful tests
https://github.com/microsoft/playwright-python
playwright by-far best framework I've used.

I'd be happy to give a hand

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

good spot, thanks. I think a simpler solution would be preferable as described above.

On playwright, maybe at some point we need e2e tests, but I'd rather postpone them as long as possible possible as they generally only act as a smoke test and slow down development significantly. Let's start with #18, python tests, and even some unit tests for the more complex bits of typescript code, and see how we get on.

python/fastui/enums.py Outdated Show resolved Hide resolved
Copy link
Contributor

@WolfDWyc WolfDWyc left a comment

Choose a reason for hiding this comment

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

This still isn't compatible, line 67 needs
def fill_fields(self) -> typing.Self: -> def fill_fields(self) -> typing_extensions.Self

I don't have permission to directly comment a line, so commenting here instead.

@WolfDWyc
Copy link
Contributor

WolfDWyc commented Dec 2, 2023

Also missing a typing_extensions import for Required in json_schema.py. I'll keep reporting.

@samuelcolvin
Copy link
Member

Good points, thanks.

I'll add some basic tests to ci and fix all this stuff some time soon

@samuelcolvin samuelcolvin merged commit 2579b18 into pydantic:main Dec 3, 2023
4 checks passed
@samuelcolvin
Copy link
Member

great, thanks so much.

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.

Issue StrEnum compatibility problem
3 participants