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

Feature Request: Typings #148

Open
ehossack opened this issue Jul 25, 2020 · 10 comments
Open

Feature Request: Typings #148

ehossack opened this issue Jul 25, 2020 · 10 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@ehossack
Copy link
Contributor

Request:
Provide typings for python

Sufficient to support libraries like mypy.
A starting point perhaps: https://mypy.readthedocs.io/en/stable/stubgen.html?highlight=generate

Not sure if this has already been considered, but I saw no record of it.
It would also make using some of your APIs more intuitive, as I have a hard time discerning return types.

Thanks!

@ppolewicz
Copy link
Collaborator

@ehossack I understand why you are asking for this, but I don't understand why. We have documentation with type annotations for more than a year now. Now that 2.7 and 3.4 are out, we can move the type annotations from docstrings to nicer syntax, but almost everything is documented.

What would change for you if the typings were provided in Python 3.5-compatible manner?

@ppolewicz ppolewicz added the enhancement New feature or request label Jul 26, 2020
@ehossack
Copy link
Contributor Author

ehossack commented Aug 1, 2020

Hi sorry for the delay @ppolewicz

What would change is tooling like mypy would be able to recognize and check the type annotations. Docstrings is usable when reading/browsing, but sometimes they are incorrect, and do not enable automated tooling to verify the validity of your calls to the SDK.

Does that make sense?

@ppolewicz
Copy link
Collaborator

ok so your project uses mypy and you'd like to have warnings for improper calls to b2sdk?

@ehossack
Copy link
Contributor Author

ehossack commented Aug 2, 2020

Yes. That's correct.

@ppolewicz
Copy link
Collaborator

All right, I now understand. The difficulty of work required to do this is not actually that high, the number of public interface classes is quite low (I'd say it's mostly B2Api, Bucket, sync and utils, maybe a couple of small classes like FileVersion).

Would you like to try to implement this @ehossack? It would be a really nice contribution. We'd then install mypy to make sure the types are correctly set inside b2sdk.

@ppolewicz ppolewicz added the good first issue Good for newcomers label Aug 3, 2020
@ehossack
Copy link
Contributor Author

ehossack commented Aug 4, 2020

Hi, I can potentially take this on, but have a couple other queued contributions to make first (will be at least a couple of weeks, if not more), so if anyone else sees this and wants to take in on first, please do so!

@ehossack
Copy link
Contributor Author

@ppolewicz one thing I've noticed is that if this projects adds type annotations, it will be incompatible with 3.5 and below. Given the next release notes indicate the next version will drop support for 2.7 and 3.4, and that 3.5 is no longer supported after this month, is that compatible with the release plans?

If so, thinking about approaching this issue out loud:

  • don't type everything at once, allow for incremental changes
  • add a type check lint step in the build? mypy sound reasonable?
  • request new contributions and additions be typed?

@ppolewicz
Copy link
Collaborator

I think we've dropped support for 2.7 and 3.4 already, please talk with @mlech-reef about adding the linter, I'm not sure which stage would be appropriate

@mlech-reef
Copy link
Collaborator

Hi.
Yes, python 2 and 3.4 are no longer supported on the master branch and we had been thinking about introducing mypy, but we put it not on the beginning of our list for CI/CD improvements. We wanted to focus first on a few other improvements which are already done and a few others which are not yet done, like:

  • replace pyflakes in favor of flake8
  • use docformatter
  • use isort

Using typing and mypy would be great, but I have few concerns:

  • typing has evolved since python 3.5, so we might need to drop support for 3.5 too as you mentioned to fully gain from the typing feature and mypy. I don't remember all the differences right now and what are the restrictions for 3.5.
  • if we start incrementaly, I don't know if we can enable mypy only for partial of the code, but we want to start using linters to prevent non-typped PRs to be deployed. I think that someone should look into https://github.com/Instagram/MonkeyType and check whether we can use it or not

If you want to start working on that, I will be more than happy to support you if you need. The mypy linter can be added to the lint nox session in noxfile.py

@mjurbanski-reef
Copy link
Contributor

This project has grown considerably since this was originally opened. A few of Python have been deprecated as well.

  • we are using ruff as linter (replacing all tools mentioned above), and use mypy-compatible type annotations for all new methods (albeit no guarantees are made in this regard)
  • we are using typing-extensions package & have updated supported Python matrix. At the moment, compatibility with Python older than 3.7 is not a concern.

We are open to contributions adding type hints for old APIs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants