Move type annotations inline

carpedm20 / emoji

emoji terminal output for Python

Other

1.87k stars 273 forks source link

Move type annotations inline #290

Closed lovetox closed 4 months ago

lovetox commented 4 months ago

This moves all type annotations inline

I did not touch any code logic
I had to place 4, 5 type: ignore on some lines because pyright thinks there are errors, this should be checked after this is merged
I added a pyright config to the pyproject.toml
pyright in strict mode checks without errors
all other folders are ignored for now (docs, util, test, examples ... )

This does not mean you need to use pyright as a type checker for this project. The config section simply allows developers who use pyright to pick up the config.

cvzi commented 4 months ago

I need a few days before I have time to look at this in detail.

Can you enable and run the github action on your forked repository to test all Python versions? You should be able to run it manually for each branch.

It fails on older Python on my machine.

It there is no easy fix, maybe we could drop support for end-of-life Python versions. We would need to check how often the package is still used on those versions at the moment (ref #243 )

lovetox commented 4 months ago

hi, all checks run now trough on all python versions

But this is only possible with a new dependency

typing_extensions which provides type annotations which are backwards compatible

If we don't want this dependency then our options are

Losing the Literal type, this would not be a big problem, as we can still type the argument as str
Lose the TypedDict and replace it with a dict[str, Any]

lovetox commented 4 months ago

We could also use the Match class from typing_extensions this will not break with 3.13, depending on the decision if you want to use typing_extensions

lovetox commented 4 months ago

PR rebased, and code review suggestions squashed

lovetox commented 4 months ago

I added some CI updates

Upgraded the actions
Added a new "lint" job, where we can gather different linting steps in the future
Its not necessary to execute linting with a python matrix, because the most linters can be configured to test with a specific version in mind (e.g. see pyproject.toml -> tool.pyright)

cvzi commented 4 months ago

Looks good to me. The typing_extensions dependency is ok with me.

@TahirJalilov I think you can merge this too, but let's wait with a new release.

I will look into solving some of these type: ignore. And also while I reviewed this PR, I found some unrelated problems with the tests/pytest that I want to solve.

lovetox commented 4 months ago

As the typing extension is accepted, i fixed the last thing that we now use the Match object from typing_extensions which should not break with 3.13

TahirJalilov commented 4 months ago

Looks good to me. The typing_extensions dependency is ok with me.

@TahirJalilov I think you can merge this too, but let's wait with a new release.

I will look into solving some of these type: ignore. And also while I reviewed this PR, I found some unrelated problems with the tests/pytest that I want to solve.

ok @cvzi Thank you!