feat(typing): add type hints

[rafalkrupinski]

Didn't use TypeAlias bc it's python>=3.10 so it would require typing_extensions (can be added if necessary).

quality_and_fitness_parsed declares rtype: (float,int) but the second value adds value of q, which may be float.

[vytas7]

Wow, we were not really planning much development here, but I agree that adding type hints would be a great addition! (We are in the process of adding type hints to the Falcon framework itself.) 👍

Thank you for this improvement! 💯

However, shouldn't we include an empty py.typed file indicating this package is typed? Not even sure how to achieve that in the current module structure... maybe we need to create a subdirectory with __init__.py like other "normal" packages do? 🤔

Furthermore, running mypy tells me there is room for improvement:

$ mypy --strict mimeparse.py 
mimeparse.py:93: error: Argument 2 to "pop" of "dict" has incompatible type "None"; expected "str"  [arg-type]
mimeparse.py:134: error: Incompatible types in assignment (expression has type "int", variable has type "Literal[100, 0]")  [assignment]
mimeparse.py:141: error: Incompatible types in assignment (expression has type "int", variable has type "Literal[100, 0]")  [assignment]
mimeparse.py:144: error: Incompatible types in assignment (expression has type "float", variable has type "Literal[100, 0]")  [assignment]
mimeparse.py:148: error: Incompatible types in assignment (expression has type "str", variable has type "int")  [assignment]

[rafalkrupinski]

Wow, we were not really planning much development here

I wanted to do #50 and it always helps to know what's being passed around.

shouldn't we include an empty py.typed file

I'll take a look what to do about py.typed file in this case. Might need to move code to mimeparse/__init__.py, like you've said.

Furthermore, running mypy tells me there is room for improvement

I did check with mypy, and the return type hints are correct, only at some point inside functions the variables don't match them.

For example a weight is initialized with 0 and then at some point a float is added, or dict value is set to None and then overridden with some other value 🤷.

I wanted to keep the change minimal so I've decided to keep it. I'm happy to change it if you're planning to add pre-commit with mypy ;)

[vytas7]

Hm, I'm no expert at typing, but I think it would be good to check Mypy's output and see if we can fix the code inside the functions too.
Otherwise, if we just care about the user experience for using mimeparse, maybe we should simply a stubs instead?

What do you think @CaselIT?

[rafalkrupinski]

Wow, we were not really planning much development here

Do you consider this package as internal to falconry? Let me know please, if so, I can always fork, so no biggie.

edit: I've changed the sentence, I wasn't too sure about how others might take it.

[vytas7]

Wow, we were not really planning much development here

Do you consider this package as internal to falconry? No problem, I can always fork.

No, not internal at all. We are actually thinking even to reimplement parts of it and unvendor it from Falcon completely.

By writing that, I just meant that this package didn't attract many contributions in the 8 years since the last release 🙂 (We took over about 3 years ago, since we are using it, even if vendored inside, as it was getting abandoned.)

[rafalkrupinski]

$ mypy mimeparse
Success: no issues found in 1 source file

[vytas7]

W00t, great that mypy passes now.

It seems that you have slightly altered the behaviour of the package's functions though; although I'm not opposed to change them if they are for good, but it would be better to track these improvement separately from typing.

[rafalkrupinski]

Yeah, I see that. I thought the changes wouldn't cause any issue, didn't think to run the tests :/

[rafalkrupinski]

I use it in my project, and I really like having less code to maintain, so...

[vytas7]

Looks good now I think, but I'd let @CaselIT take a look at this before merging.

[vytas7]

LGTM thanks!