Python return types

59 views
Skip to first unread message

John H Palmieri

unread,
Sep 6, 2023, 1:24:06 PM9/6/23
to sage-devel
For a while now, Python has allowed return types to be specified in signatures:

    def f() -> int:

It is my understanding that this doesn't actually do any error-checking or type-checking — see https://docs.python.org/3/library/typing.html. So this works without error:

    def f() -> float:
        return "hello"

This is cropping up in the Sage library. The file combinat/permutation.py has several methods that claim (according to their signature) to return an `Integer` but actually return an `int`, and this has led to at least one person being confused (a recent post on stackexchange cited in the github issue). Anyway, I've opened https://github.com/sagemath/sage/issues/36198 for one of these instances, and we should be aware of this.

--
John

Dima Pasechnik

unread,
Sep 6, 2023, 2:02:15 PM9/6/23
to sage-...@googlegroups.com
Yes, one need mypy, most probably.
E.g.

$ pip install mypy --user
$ cat t.py

def f() -> float:
        return "hello"
$ mypy t.py
t.py:2: error: Incompatible return value type (got "str", expected "float")  [return-value]
Found 1 error in 1 file (checked 1 source file)

No idea how it plays with cython, though.

Should we run mypy on all the *.py files, at least, in our CI?



--
You received this message because you are subscribed to the Google Groups "sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/63bea4e5-ba57-4ddf-b59c-b46d59e3505bn%40googlegroups.com.

Oscar Benjamin

unread,
Sep 6, 2023, 2:11:44 PM9/6/23
to sage-...@googlegroups.com
We have had this issue in SymPy as well. Contributors want to add the
type hints because they think it is good practice and many Python
editors even suggest these hints automatically now. The hints are
actually very useful for end users because editors can interpret them
and use them for autocomplete etc. I showed some examples here:
https://github.com/sympy/sympy/pull/25103

The hints are also useful for picking up bugs if used uniformly
throughout the codebase but it is difficult to get that benefit
incrementally in a large codebase with lots of legacy code.

The hints are worse than useless if they are not correct though.
Without any checking the hints are very likely to be incorrect. I
suggest disallowing hints unless there is actually a type-checker
running that will reject incorrect hints. When I enabled mypy in
SymPy's CI almost all of the work involved was fixing all of the
incorrect type hints that had already been added so I suggest not
allowing those to build up.

I also suggest choosing pyright over mypy if wanting to use a
typechecker because it is more principled and consistent (basically
just better AFAICT) and also pyright is what is used in all the
editors that are the main consumers of the hints anyway.

--
Oscar

Matthias Koeppe

unread,
Sep 6, 2023, 4:45:10 PM9/6/23
to sage-devel
On Wednesday, September 6, 2023 at 11:02:15 AM UTC-7 Dima Pasechnik wrote:
Should we run mypy on all the *.py files, at least, in our CI?

We already have "sage --tox -e pyright", which is considered a better choice than mypy.

We are also running it in the GitHub CI as part of the Build & Test workflow.

John H Palmieri

unread,
Sep 6, 2023, 6:21:25 PM9/6/23
to sage-devel
I tried using mypy, and I also just ran "sage --tox -e pyright", and neither seems to flag the conflict between the signature "def signature(self) -> Integer:" and the output, which is some arithmetic with "len(...)" and hence a Python int. (And both mypy and pyright produce a ton of warnings, so if either is managing to flag anything worth fixing, it's likely to get drowned out. Or maybe I'm using them wrong...)

Tobia...@gmx.de

unread,
Sep 7, 2023, 1:39:53 AM9/7/23
to sage-devel
Currently, the biggest problem for the type checker is that Cython is not emitting any type annotations, so pratically all cython classes are `any` (which is probably also the reason why the Integer return value is not flagged). For now, one would need to generate the `pyi` stub files (using some tool) and put them in version control, see https://github.com/cython/cython/pull/3818. Oscar, how is that currently solved in sympy? Other then that, the typing annotations in sage are at the very beginning: relative to https://microsoft.github.io/pyright/#/getting-started, we are now at step 2. I tried to add many typings in the manifold package, but even that effort is not yet completed (and is made harder by the fact that everything else is untyped).

Of course, help in improving the situation is more than welcome!
Reply all
Reply to author
Forward
0 new messages