Proposal for Python type annotations in Qpid Proton and Qpid Dispatch

[Jiri Daněk <jd...@redhat.com>]

Hi Folks!

Having dropped Python 2.7 support in Qpid Proton and Qpid Dispatch recently
(see thread "Some (long awaited?) suggestions to deprecate Python 2 and C++
03 support" from Andrew Stitcher), I propose to introduce Python type
annotations into the codebase.

Optional typing syntax was added in Python 3.5 and Python 3.6. The syntax
is (subjectively speaking) quite ugly (many people think that,
https://lwn.net/Articles/643269/). On the other hand, automatically
typechecked type annotations provide numerous benefits to the users:

  * Up-to-date documentation: oftentimes, type signatures give strong usage
hints when trying to use an API. Am I allowed to pass None here? When the
type annotation says Optional[str], the answer is clear. Can this method
return None?
  * Code completion and linting: There is IDE support as well as standalone
static type checkers for Python type annotations. The IDE uses the
annotation information for code autocompletion. Both the checkers and IDEs
report type mismatches in code.

The best single-source introduction to optional types in Python is probably
the MyPy documentation,
https://mypy.readthedocs.io/en/stable/introduction.html.

One example of AMQP 1.0 client (for JavaScript) that ships typing stubs is
https://github.com/amqp/rhea, contributed in
https://github.com/amqp/rhea/pull/70.

# Inline or stubs?

There are two possibilities for writing type annotations in Python. Either
stubs in separate .pyi files, or inline annotations in function definitions.

The stubs approach allows to keep the ugly typing syntax out of the main
sources. Historically it was the approach used when Python 2.7
compatibility had to be maintained or when the types were being written by
somebody other than the library author. To simplify working with stubs,
PyCharm IDE draws a star gutter icon for module members annotated in a
separate stub file that allows switching back and forth.

I feel that stubs would make some sense for Qpid Proton, and less sense for
Qpid Dispatch. Proton is a library defining a public API, changes to which
happen infrequently. Stubs allow to hide the syntactic ugliness. On the
other hand, inline types are easier to work with as it is not necessary to
switch between files much. From a library user's point of view, the chosen
approach does not matter.

Over all, my preference goes to inline type annotations, simply because
they are easier to write and maintain.

Due to the notorious ugliness of the inline syntax, a reformat of parameter
lists may be needed. As an example, going from

def f(self, a, b=None, c=42):
    ...

to

def f(
    self,
    a: Union[A, B, C],
    b: Optional[str] = None,
    c: int = 42
) -> Dict[str, Any]:
    ...

See https://github.com/sphinx-doc/sphinx/issues/5868 for one more hairy
example.

# Which checker?

  * https://github.com/python/mypy

  * https://github.com/facebook/pyre-check
  * https://github.com/google/pytype
  * https://github.com/microsoft/pyright

I believe MyPy should be supported at first, and one other checker can be
added later. The checkers all use the same type syntax, but they differ in
how they do type inference and therefore what errors they report on a
particular piece of code. Code which passes MyPy might fail on Pyre, for
example.

https://google.github.io/pytype/faq.html#how-is-pytype-different-from-other-type-checkers

# Initial annotations proposal

I propose to generate initial type annotations by capturing runtime types
using https://github.com/Instagram/MonkeyType, introduce them inline, and
edit out the most egregious artifacts that this method produces (i.e. large
Union types where the algorithm fails to divine the programmer's intention
and does not generalize the type). In a subsequent commit, I propose adding
a MyPy CI job and finetune the types and write more of them so that the
check passes. Finally, I'd like to attempt to make MyPy check pass in
--strict mode.

Having typechecking in place should be helpful for PROTON-2095 Move away
from SWIG to CFFI, in order to exactly preserve the API.
-- 
Mit freundlichen Grüßen / Kind regards
Jiri Daněk

Re: Proposal for Python type annotations in Qpid Proton and Qpid Dispatch

[Jiri Daněk <jd...@redhat.com>]

PR: https://github.com/apache/qpid-proton/pull/325

I've focused on simple and mostly obvious declarations there. I want to get
them merged first, and then tackle the ones where e.g. current
documentation does not match the actual types. While working on the PR, I
encountered some less obvious features of Python typing, which I'd like to
briefly mention in the remainder of the email. Chances are that if you are
new to Python typing, you haven't encountered them yet.

# if TYPE_CHECKING

Importing a file just to get type definition sometimes leads to circular
imports. Conditional import like this breaks the cycle for regular
execution; when type-checking, the checkers can deal with the import
cycles, since they are not actually executing the checked code.

# from __future__ import annotations

This is an useful Python language extension, that can be opted into
starting with Python 3.7. When enabled, it stops Python from trying to
evaluate the inline type annotations as it is loading each file. This
allows dropping the single quotes for types that are only defined later in
the file, or are conditionally imported.

# @overload

This allows multiple type signatures for a method. For example, _check_type
does casting in the following way: If an argument of type symbol or str is
given, the function will cast it to a symbol. If allow_ulong is True, then
also ulong return value is possible. If raise_on_error is False, then any
type is accepted and returned. That leads me to a crazy type definition
along the following lines. This is not something that I added to a PR just
yet; I am now focusing on short, sweet and useful types, for now.

_T = TypeVar('_T')

@overload
def _check_type(s: Union[symbol, str], allow_ulong: bool = ...,
raise_on_error: bool = ...) -> symbol:
    ...


@overload
def _check_type(s: Any, allow_ulong: Literal[False] = ..., raise_on_error:
Literal[True] = ...) -> Union[symbol]:
    ...


@overload
def _check_type(s: Any, allow_ulong: Literal[True] = ..., raise_on_error:
Literal[True] = ...) -> Union[symbol, ulong]:
    ...

@overload
def _check_type(s: _T, allow_ulong: bool = ..., raise_on_error:
Literal[False] = ...) -> _T:
-- 
Mit freundlichen Grüßen / Kind regards
Jiri Daněk