django-ftl/fluent-compiler

Provide Type Annotations (PEP 484)

samineer opened this issue · 9 comments

samineer commented

Reasons

  • Improving developer experience with this library.
  • Possibility to use static type analysis for the code base.

Introduction of Type Annotations

  • Do some refactoring.
    • #19
    • #21
    • #22
    • #23
    • Exclude some modules from the package's public API.
    • ...
  • Get mypy to run successfully.
  • Add type annotations for:
    • ...

References

spookylukey commented

Hi @Samylov-Mikhail thanks for opening the issue!

In general I think there is a good chance that type annotations and mypy checking (or maybe pyright) could be useful in this library, although I think they are not always helpful. It depends what you are using them for and the nature of the library.

In this case I've got a few questions for you:

  • what is your particular interest in fluent-compiler?
  • which of the different uses of type hints concern you?
  • Are you thinking from the perspective of a user of fluent-compiler or a prospective maintainer on the code base? This question is quite important, because the user interface of fluent-compiler that I expect is actually in use is very small - basically https://fluent-compiler.readthedocs.io/en/latest/api/compiler.html and a few other bits and pieces. This is the interface that django-ftl uses, for example, and no-one using django-ftl uses fluent-compiler directly. I currently have extremely little info on who else is using this library directly, but I expect it is very few people. The maintainer perspective on types is probably very different.

All of the above questions are another way of asking "what problem are you hoping to solve?"

spookylukey commented

Sorry, I didn't mean to close!

samylovma commented

Hi, Luke. Thank you for the quick feedback!

What is your particular interest in fluent-compiler?

I'm using fluent-compiler directly (not using django-ftl) in my chat bot application. I have chosen this library because of better maintenance compared to fluent.runtime.

Which of the different uses of type hints concern you?

Mostly static type checking to detect issues with my use of the library.

Are you thinking from the perspective of a user of fluent-compiler or a prospective maintainer on the code base?

Mostly I thinking from the perspective of a user. Type hints can improve UX for developers like me who use fluent-runtime directly.

Since the library is stable, I guess type hints won't be enemies of a prospective maintainer. Otherwise we can create third-party type stubs.

Benefits of type hints for a maintainer:

  • specifying the interface of the library;
  • using static type checkers to find issues.

Have you considered pyright instead of mypy?

Seriously no. I have no real experience with pyright. In mypy compared to pyright I like cool documentation and large community.

What kind of approach would you be thinking of to add types? Have you looked at https://github.com/Instagram/MonkeyType or similar?

Since the codebase isn't so large I'm thinking of adding type hints manually. But I don't mind trying automated tools for that.

spookylukey commented

That's sounds great - as long as you are a user and looking at changes that would benefit you as a user, that's great. My only concern is adding lots of stuff that may or may not be helpful. As a maintainer, there are definitely bits I know of that could benefit from types, but there are also bits that would be very hard to type and I wouldn't want you getting stuck with something too ambitious!

samylovma commented

Thank you for your rationality, @spookylukey!
I suggest just starting to integrate mypy and add type hints gradually where it's real helpful.
What do you think about this approach?

spookylukey commented

Thank you for your rationality, @spookylukey! I suggest just starting to integrate mypy and add type hints gradually where it's real helpful. What do you think about this approach?

That sounds fine to me - thanks in advance!

samylovma commented

Good!

So I have a plan:

  1. Do some refactoring before the next step. (I will create an issue or a PR for every point.)
    • #19
    • #21
    • Drop support for Python 3.6.
    • Exclude some modules from the package's public API.
    • Fix all warnings in the tests.
    • ...
  2. Get mypy to run successfully.
  3. Add type hints for:
    • ...

P. S. This plan can be updated.

UPDATE: This plan moved to the body of this issue.

spookylukey commented

Remove the ast_compat module.

Is there a reason for this? What particularly problem would removing it solve? The Python AST is not stable, and I expect it will continue to have changes, and having a single place to accommodate those changes has been very useful in the past.

samylovma commented

@spookylukey, I've created the issue #19 for that.