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

Introduce types in the whole package #181

Open
alecandido opened this issue Dec 16, 2022 · 3 comments
Open

Introduce types in the whole package #181

alecandido opened this issue Dec 16, 2022 · 3 comments
Labels
benchmarks Benchmark (or infrastructure) related documentation Improvements or additions to documentation enhancement New feature or request refactor Refactor code
Milestone

Comments

@alecandido
Copy link
Member

At the moment, type hints has been mainly introduced in eko.io, and inconsistently here and there in other places.

We should consistently define and use types, as a starting point we should better distribute types that are common to the whole package, lifting a huge part of eko.io.types to eko.types.

This will be also helpful for #178

First discussed in #172 (comment)

@felixhekhorn felixhekhorn added documentation Improvements or additions to documentation enhancement New feature or request refactor Refactor code benchmarks Benchmark (or infrastructure) related labels Dec 19, 2022
@alecandido alecandido added this to the post milestone Jan 27, 2023
@alecandido
Copy link
Member Author

Despite being user-facing, and somehow part of the interface, types are optional in Python. I'll consider them as a non-breaking change, at least for the time being.

@felixhekhorn
Copy link
Contributor

Out of the suggested tools here https://mypy.readthedocs.io/en/stable/existing_code.html#automate-annotation-of-legacy-code only MonkeyType seems a realistic option

@alecandido
Copy link
Member Author

Do not use any automated option: types are useful to add information, while, if the information is already there, you could leave it untyped. This is perfectly fine for Python (to have types only somewhere), since Any is implicitly added.

I don't remember what's the default (I could quickly look up), but Mypy for sure has an option to (dis)allow Anys.

So, let's keep doing it incrementally. If you want to add a bunch of them manually, feel free to do it. But even in that case, please break it in multiple PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmarks Benchmark (or infrastructure) related documentation Improvements or additions to documentation enhancement New feature or request refactor Refactor code
Projects
None yet
Development

No branches or pull requests

2 participants