Skip to content

Consider type hints for 3.x.y release? #213

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

Closed
tomschr opened this issue Dec 10, 2019 · 15 comments
Closed

Consider type hints for 3.x.y release? #213

tomschr opened this issue Dec 10, 2019 · 15 comments
Assignees
Labels
Question Unclear or open issue subject for debate Release_3.x.y Only for the major release 3

Comments

@tomschr
Copy link
Member

tomschr commented Dec 10, 2019

Situation

Python 3.5 introduced the typing module. As soon as we are sunsetting the 2.9.x release of semver, it raises the question if we should use type hints for our 3.x.y release of semver.

Currently, we don't use it for our functions and methods.

To avoid any misinterpretations, the typing module contains this note:

The Python runtime does not enforce function and variable type annotations. They can be used by third party tools such as type checkers, IDEs, linters, etc.

To start discussion and to get a better picture where we stand on this topic, I've opened this issue. 😉

Example

This is a short example of a type hint for the parse function:

def parse(version: str) -> dict:
    """
    Parse version to major, minor, patch, pre-release, build parts.

    :param version: version string
    :return: dictionary with the keys 'build', 'major', 'minor', 'patch',
     and 'prerelease'. The prerelease or build keys can be None
     if not provided

Bernát Gábor recommends that “type hints should be used whenever unit tests are worth writing.”

Pros and Cons

I've collected some pros and cons, probably there are more that I haven't listed here. This list is by no means exhaustive. Feel free to amend it.

Pros

  • Helps to catch type errors when testing with mypy.
  • Could support IDEs to provide a better popup help.
  • Improves documentation when using the sphinx-autodoc-typehints plugin.
  • Help you as a developer to write better code.

Cons

  • A slight impact of performance.
  • Needs another dependency when checking for type hints (mypy).
  • Needs a bit more time to add it.
  • Works best with modern Python (version >= 3.5).

Scope

If we want this, it will only be implemented in the 3.x.y release of semver. Older releases (2.9.x) are not covered by this issue and won't be modified.

Furthermore, we need to create a semver.pyi file with the command stubgen (integrated in mypy).

See also

@tomschr tomschr added Release_3.x.y Only for the major release 3 Question Unclear or open issue subject for debate labels Dec 10, 2019
@tomschr tomschr self-assigned this Dec 10, 2019
@ppkt
Copy link
Member

ppkt commented Dec 10, 2019

I'm a huge fan of typing in Python. I wasn't aware about impact on performance but I think every library should use them. As far as I know, Python 3.4 is no longer maintained so we can drop support for it.
I don't use mypy - I only rely on support in IDE but results are really great, especially in bigger projects.

I think it's a must-have feature for semver 3.0.0 :)

@tomschr
Copy link
Member Author

tomschr commented Dec 10, 2019

Thanks Karol for your feedback! 👍

The performance depends probably on how difficult the types are and how big the library is. I've read somewhere an article about it. If I remember correctly, the performance penalty isn't big, but measurable.

If we use that, I would suggest to add mypy as an additional check besides black/flake8/docstrings.

@jwarner112
Copy link

I'm just acclimating myself to static typing now and mypy is currently complaining that semver has to typing info.

To minimize the amount of overhead involved in this, it's worth noting there are several ways to get the typing info, among which are:

  • Adding type hints to semver
  • Creating a complimentary package for type hinting, semver-stubs or somesuch
  • Adding type hints to the third-party section of python/typeshed

Anyway I'm really eager to see this added into semver 3 and am rooting for you all the way! 👏

@tomschr
Copy link
Member Author

tomschr commented Jun 16, 2020

@jwarner112 Many thanks Jeff for your motivating comment and your useful hints. 👍

Do you have any recommendations which of the two (semver-stubs or typeshed) are useful?

I vaguely recall that a stubs package is easier to create than adding the information to typeshed. If I remember correctly, you have more control when creating a stubs package than adding your changes to typeshed (and waiting to be integrated).

Probably my information is incomplete or inaccurate. If you have any hints or tips which could be helpful for this project, I would be very grateful for your help.

@jwarner112
Copy link

jwarner112 commented Jun 24, 2020

I can't say for sure which is better, but I can definitely say that having your own -stubs package would be easier for you due to the control it gives you. With added responsibility, of course 🕷️. I'd probably recommend that, given your situation. I get the idea that typeshed is more for packages that are third-party but so popular/close to the core team that it might be considered second party? Not formally, of course, but in practice.

Edit: As for the implementation of typing stubs themselves, I have absolutely no idea. If my work comes across it though and I have to figure it out, I'll remember to come back here and update.

@sbrudenell
Copy link
Contributor

I'm having trouble finding evidence that type hints have any non-trivial runtime performance penalty.

The best argument I can find is that importing the typing module increases startup time, which is technically true, but can also be said of code comments, longer descriptive identifier names, or organizing semver into a package.

Adding to @jwarner112 's list of alternatives: you can also put type stubs in a separate *.pyi file in semver, next to the *.py files; no -stubs package is necessary. I think this approach may force you to make semver a package, but I'm not sure.

Anecdotally, I find *.pyi files to be a nice concise reference. I often refer to the *.pyi files in typeshed rather than read the standard library documentation.

@tomschr
Copy link
Member Author

tomschr commented Oct 21, 2020

I'm having trouble finding evidence that type hints have any non-trivial runtime performance penalty.

I've read it somewhere, but can't remember where I've found it. It wasn't much, but it was just a tiny overhead.

ATM I'm working on the 3.0.0 release and I'm publishing the first alpha version soon. Maybe I can add the type hints in one of the next alphas or betas.

@tlaferriere
Copy link
Contributor

I've created tomschr#6 with type hints directly in the code. Do you think I should move them to a *.pyi file instead?

@tomschr
Copy link
Member Author

tomschr commented Oct 22, 2020

I don't have much experience with type hints yet. From what I've seen, I think a *.pyi file would be worth. So yes, if this is not too much work, please go ahead! 👍

@sbrudenell
Copy link
Contributor

The two main functional uses of *.pyi files are

  1. type hinting a c extension
  2. type hinting python code targeted for python <3.5

For python <3.5 support, you can also add type hints via inline # type: ... comments, but in practice I don't find these to be very readable.

Not sure which python versions you intend to support in semver 3. But strictly speaking, if you only want to support non-EOL python versions (>=3.6), you don't need a *.pyi file, and could just have type hints inline in the code. Personally, I find inline type hints improve readability.

@tomschr
Copy link
Member Author

tomschr commented Oct 22, 2020

Thanks @sbrudenell for your input. 👍 We currently would like to support Python >=3.6.

I was under the impression, the *.pyi files are also used for IDEs and editors as type annotations? This would have the benefit that IDEs/editors don't have to parse the complete file. Or am I completely wrong?

@tlaferriere
Copy link
Contributor

Well on pycharm, the inline types are parsed quite well, and it also tries its best to infer type of other parts in the code. I agree with @sbrudenell on the fact that having them inline makes the code more readable, so if you have a more basic editor that doesn't read any type hints, having to open the *.pyi alongside it would be much less ergonomic.

@tomschr
Copy link
Member Author

tomschr commented Oct 23, 2020

Ok, I had to edit my own comment. I have overlooked some of the comments.

So it seems we don't need a .pyi file. As we don't support Python <3.6, this is probably not necessary.

@tomschr
Copy link
Member Author

tomschr commented Oct 26, 2020

As Thomas Laferriere already added type hints into #290, I think we can consider this as done now.
Thanks Thomas for all your work! Much appreciated! 👍

@tomschr tomschr closed this as completed Oct 26, 2020
@sbrudenell
Copy link
Contributor

Inline type hints LGTM. Just to clarify:

I was under the impression, the *.pyi files are also used for IDEs and editors as type annotations? This would have the benefit that IDEs/editors don't have to parse the complete file. Or am I completely wrong?

*.pyi files are just an alternate delivery mechanism for hints; they should be equivalent to inline hints and editors/etc should treat them so. They also support full python syntax so I don't expect there's a speed benefit for a parser to visit a *.pyi versus a *.py.

I probably caused too much confusion by bringing up *.pyi files at all. I think inline hints are best, but I expected the authors to disagree, so I prematurely offered a compromise.

tomschr added a commit to tomschr/python-semver that referenced this issue Nov 1, 2020
tomschr added a commit to tomschr/python-semver that referenced this issue Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question Unclear or open issue subject for debate Release_3.x.y Only for the major release 3
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants