ansible / proposals

Repository for sharing and tracking progress on enhancement proposals for Ansible.
Creative Commons Zero v1.0 Universal
93 stars 19 forks source link

Static type checking in Ansible #44

Open xen0l opened 7 years ago

xen0l commented 7 years ago

Proposal: Static type checking in Ansible

Author: Adam Stevko <@xen0l>

Date: 2016/12/18

Motivation

Ansible source code has grown over the years and is changing rapidly. The code base is getting bigger, it's harder to spot bugs and readability might suffer in certain cases. By introducting type hints, we can eliminate some of these points.

Problems

What problems exist that this proposal will solve?

Solution proposal

from typing import List

def hello(): # type: () -> None
    print 'hello'

class Example:
    def method(self, lst, opt=0, *args, **kwargs):
        # type: (List[str], int, *str, **bool) -> int
        """Docstring comes after type comment."""
        ...

Dependencies (optional)

Type hints by itself do not introduce any runtime or build time dependency. However, to do the static linting, mypy has to be run during build time.

mypy needs Python 3 and for Python 2 'typing' package has to be available.

Documentation (optional)

https://mypy.readthedocs.io

Anything else?

mypy success story on large codebase

gundalow commented 7 years ago

@mattclay @abadger Sounds interesting

mattclay commented 7 years ago

@xen0l I've added this proposal to the agenda for the next core team meeting on IRC. Do you think you'll be able to attend?

gundalow commented 7 years ago

Discussed in Core Meeting 2016-12-20

Agreed

xen0l commented 7 years ago

@mattclay Sorry that I couldn't make it to the core team meeting. Few notes:

May get out of date when someone updates a function

The same problems is also present for docstrings and documentation. It is usually a matter of review / tests. If function is extended with a new argument and mypy is run, it will complain, so this will be rather easy to spot.

May be too high a bar for Modules Unsure if we can implement this on just certain files, or if it has to be everything

i wouldn't go that far right from the beginning and leave modules out. Type hints can be only on certain files and not on everything. However, to achieve best results, it should be used everywhere.

Have we seen bugs that this would have caught?

I have been closely watching Ansible development for last couple of months and I am not aware of any bugs. Type hints should help catching typeerrors for example.

Unsure on cost or benefit

There is no performance cost while Ansible is executed. As for other costs, I can see only maintenance costs. I expect it to have the same maintenance cost as docstrings. Only other cost is a need for python3 why building as mypy requires it. There will also be a new dependency on "typing" module for Python 2.7, for Python 3 typing module is built-in since 3.

Apart from benefits mentioned in "Benefits of using mypy" part from http://blog.zulip.org/2016/10e/13/static-types-in-python-oh-mypy/:

According to the same blog post, there are few dowsides, which stems from other linters not supporting everything from mypy yet. There should be more research done on this topic.

i (abadger) think in terms of maturity static typing will either be much more mainstream or more obviously niche in two years time.

If we look at trends in other interpreted programming languages (Hack, Flow, Typescript..) and Python itself, static typing will mature and will be much more widespread. Also the fact that Python BDFL himself is working on this makes me certain it won't be niche.

I am interested on working on this and could start with a small PoC if there is an interest from the core team.

mattclay commented 7 years ago

@xen0l There's definitely interest. Where were you thinking about starting for a PoC?

xen0l commented 7 years ago

@mattclay I have already started some small PoC. I plan to work on a file with few simple functions in it and on also on something more complex with different imports from lib/ansible. I will keep you updated (I will be mostly away from computer in the upcoming days).

abadger commented 7 years ago

the new dependency may be a blocker for this. Can the dependency be required during testing and not installed for general use?

abadger commented 7 years ago

If we look at trends in other interpreted programming languages (Hack, Flow, Typescript..) and Python itself, static typing will mature and will be much more widespread. Also the fact that Python BDFL himself is working on this makes me certain it won't be niche.

Many things the BDFL works on are niche.

I am interested on working on this and could start with a small PoC if there is an interest from the core team.

I'm for evaluating and possibly adding it in 1.5 to 2 years time (assuming the library dependency is optional). Adding type hints before it becomes widespread raises the contribution barrier to entry significantly and I would not want to do that to our other contributors.

We're even struggling with docstrings right now... docstrings themselves are a widespread practice in the python community and most contributors understand them. Sphinx is a widespread format for docstrings but most contributors do not understand it. This means that the burden of adding and maintaining the docstrings has mostly fallen on us (I don't know of anyone who's added sphinx docstrings to core code besides myself, actually).

xen0l commented 7 years ago

the new dependency may be a blocker for this. Can the dependency be required during testing and not installed for general use?

@abadger typing module needs to be installed as it we need to import from it in the source code. Unfortunately, "pip install typing" is a requirement on Python 2, but on Python 3.5+ it is in stdlib. If additional package for Python 2 is considered a blocker, then I'm afraid that type hints will never be adopted with such stance. I don't want to open Python 2 vs Python 3 debate, but given Ansible is slowly moving to Python 3 (and typing module is part of it), couldn't this be also considered for Python 2?

sivel commented 7 years ago

Couldn't we utilize stub files for typing? Using stub files shouldn't cause all users to have typing as a dependency.

https://www.python.org/dev/peps/pep-0484/#stub-files

As documented in the link I provide, stub files are useful for:

Modules that must be compatible with Python 2 and 3

xen0l commented 7 years ago

Many things the BDFL works on are niche.

@abadger is this your opinion or do you have some facts that could justify it?

I'm for evaluating and possibly adding it in 1.5 to 2 years time (assuming the library dependency is optional). Adding type hints before it becomes widespread raises the contribution barrier to entry significantly and I would not want to do that to our other contributors.

Typing syntax is really easy to pick up and anyone who can write Python can write type hint (as he is aware what type function argument and return value has to be) e.g.

from typing import AnyStr
def example_function(str1, str2):
# type: (AnyStr, AnyStr) -> AnyStr
return str1 + str2

If people are unsure about the type hints, the worst case scenario would be that people use type "Any" and mypy would consume. There will be some added value even if not 100% (mypy will warn for cases when no return value is given for example when somebody refactored some function etc.)

We're even struggling with docstrings right now... docstrings themselves are a widespread practice in the python community and most contributors understand them. Sphinx is a widespread format for docstrings but most contributors do not understand it. This means that the burden of adding and maintaining the docstrings has mostly fallen on us (I don't know of anyone who's added sphinx docstrings to core code besides myself, actually).

Well, if people add code, but don't provide docstrings, why Ansible accepts such PRs in the first place? To me this sounds like incomplete part of the contribution process, which can be easily caught during PR review phase. As for type hints, the situation is eventually the same, but with small difference and that people don't have to spend a lot of time of describing what the function does and writing proper documentation - type hints tell only what types the function expects and what type should be returned.

abadger commented 7 years ago

Just take a look at the things that GvR has in the PEP index: "Extensions to the pickle protocol", "Conditional Expressions" "Stop iteration inside generators" "asyncio" "type hints" ;-) .... Guido works on things that he finds interesting or that he finds necessary. Sometimes those are also interesting and necessary to the rest of the world. other times they're only interesting and necessary to a subset of python programmers and thus niche.

Adding the new dependency is almost certainly a blocker. I've asked about adding various other dependencies in the past and they've always been shot down. But you can ask as well... they're case-by-case but if run-time feature enhancements fail to pass the bar for adding deps I don't see a bright future for a testing enhancement. If sivel's note about stubs works, then that will remove this blocker.

Your whole argument about type hints being easy glosses over the problem: "Typing syntax is really easy to pick up". The problem is that it has to be "picked up" at this point. It's not something that someone can be expected to know before they contribute. That's why I think our adoption should be re-evaluated in the 2 year time frame. If we do this now, we'd be early adopters. And therefore we'd be suffering the problem of not having contributors who know how to use the hints and a rich, worldwide community of other devs who can help to train contributors. We have enough difficulty in opening up the process of contributing without introducing this at this time. Give it two years and we'll either find that the time is ripe, contributors are experienced with it, and many code bases have it to use as examples, or we'll find that the python development community hasn't really adopted type hints so it would still be a constant struggle to implement them.

xen0l commented 7 years ago

@abadger points taken, I will check out @sivel's proposal during the weekend and see if I can get it up and running. I know that stub files can be generated via stubgen (mypy) utility.

gundalow commented 7 years ago

@xen0l Let us know when their is something to discuss/show by adding it to the Core Meeting agenda https://github.com/ansible/community/issues?utf8=%E2%9C%93&q=is%3Aissue%20is%3Aopen%20label%3Ameeting_agenda%20

Thanks

bendem commented 2 years ago

I see this has been discussed again in 2021. Was there anything new added? typing is well established now and its benefits are well recognised: catching type errors earlier in the development process, allowing for refactoring that would be impossible without them, clearer API interfaces.

Simple example: I've been using the inventory module to do some checks without re-inventing parsing. It took me a while to figure it out because everything is implicit and tools cannot tell me if what I'm writing makes sense.

Type hint improves readability and discoverability, making the barrier of entry lower for new contributors.

Notice below how vscode cannnot tell that some fields are actually functions (white instead of yellow), and thus cannot provide any autocomplete.

Without: image

With: image

bcoca commented 2 years ago

Sorry for not updating the ticket, this was logged in the IRC meetings but we should have still reflected it here.

Yes, it was agreed to and we are currently adding typing, but we are not going to do this all at once, but piecemeal as we update the code. Also, we are limited to 'core code' so modules/module_utils will still have to wait until they drop some of the currently targeted Python versions.

bendem commented 2 years ago

Thanks, this is great news!