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

Provide typing stubs #40

Closed
khvn26 opened this issue Apr 18, 2023 · 8 comments
Closed

Provide typing stubs #40

khvn26 opened this issue Apr 18, 2023 · 8 comments
Assignees

Comments

@khvn26
Copy link
Member

khvn26 commented Apr 18, 2023

The following is mypy output when using Flagsmith SDK in a strictly typed project:

src/example_service/components/flagsmith.py:3: error: Skipping analyzing "flagsmith": module is installed, but missing library stubs or py.typed marker  [import]
src/example_service/components/flagsmith.py:4: error: Skipping analyzing "flagsmith.flagsmith": module is installed, but missing library stubs or py.typed marker  [import]

Hopefully, shouldn't be too hard to fix. (Famous last words before jumping in the Python typing rabbit hole.)

@tushar5526
Copy link
Contributor

tushar5526 commented Nov 23, 2023

Let's do this. Any. preference on how you want me to approach this, a single PR or different PRs module-wise. Any other things I should keep in mind before doing it?

@matthewelwell
Copy link
Contributor

@tushar5526 I think a single PR should be fine, there isn't a lot of code in this repository. The only thing that might provide a little painful is the typing of flag values but you can check the typing approaches in the flagsmith-engine repository for approaches on that - it looks like we're just using typing.Any there.

@khvn26
Copy link
Member Author

khvn26 commented Nov 24, 2023

@tushar5526 I have set up the necessary tooling and CI here: #70

Please use the feat/mypy branch as your base one. The acceptance criteria is the passing CI workflow.

Note that the SDK supports Python 3.8 so be sure to take it into account by using e.g. typing.Dict instead of dict and typing_extensions.TypeGuard instead of typing.TypeGuard.

@tushar5526
Copy link
Contributor

@tushar5526 I have set up the necessary tooling and CI here: #70

Please use the feat/mypy branch as your base one. The acceptance criteria is the passing CI workflow.

Note that the SDK supports Python 3.8 so be sure to take it into account by using e.g. typing.Dict instead of dict and typing_extensions.TypeGuard instead of typing.TypeGuard.

Sure thanks!

@tushar5526
Copy link
Contributor

Should there be type hints for tests as well?

@matthewelwell
Copy link
Contributor

Should there be type hints for tests as well?

Yes, please!

@tushar5526
Copy link
Contributor

I think this one is closed!

@rolodato
Copy link
Member

rolodato commented Apr 9, 2024

Closing, released as ed535de in 3.6.0 :)

@rolodato rolodato closed this as completed Apr 9, 2024
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

No branches or pull requests

4 participants