ENB: PR #4767 embodies lessons learned

7 views
Skip to first unread message

Edward K. Ream

unread,
Jul 1, 2026, 9:42:04 AM (2 days ago) Jul 1
to leo-editor
​I have just created PR #4767. It requires mypy's strict_optional setting for leoNodes.py.

​Per recent discussions here, this PR takes a much more careful approach to improving Leo's annotations.

Infrastructure

The first commit pays close attention to the mypy infrastructure, that is, .mypy.ini and leo/scripts/mypy_leo.py.

Experiments show that telling mypy to follow imports (follow = False in mypy_leo.py) makes little difference:

follow= False: mypy reports 24 errors, all in leoNodes.py.

follow=True:  mypy reports 25 errors in leoNodes.py. mypy also reports errors in various Qt plugin files, but the PR will ignore those!

Make haste slowly

This PR will change only two files: leoCommands.py and leoNodes.py. 

The PR's first commit follows the spirit of Thomas's suggestion to update c.currentPosition and c.rootPosition in a separate PR. This will be the only significant code change in the entire PR.

I'll wait for Thomas's and Félix's approval before merging the PR into the "devel" branch. The work on leoCommands.py is already complete. I'll complete work on leoNodes.py soon.

As Thomas says, there is no rush. We can take several days to discuss the implications of this PR.

Ahas about type checking

All the work in the abandoned PR has greatly improved my intuition about how mypy works. And about type checking in general.

Aha: It's fine to allow x: whatever | None = None in method arguments. mypy will check to make sure that the method's code never calls x.y where x might be None. Experience shows casts (or minor local reorgs) are all that are usually needed to pass mypy's muster. In short, type widening is harmless in method arguments.

Otoh, type narrowing is excellent in method returns. The changes to c.rootPosition and c.currentPosition open up this possibility throughout Leo. This has the potential to become a virtuous cascade. We'll see how this pans out.

Summary

PR #4767 embodies the lessons learned from previous work:

- It is strictly limited in scope. It changes only two core files.
- Except for two commander methods, it will make no significant code changes. Otherwise, the PR will change only annotations.
- The PR will embrace type widening for method args and will attempt type narrowing for method returns.
- I'll wait as long as necessary before merging this PR.

Edward

Thomas Passin

unread,
Jul 1, 2026, 10:39:15 AM (2 days ago) Jul 1
to leo-editor
On Wednesday, July 1, 2026 at 9:42:04 AM UTC-4 Edward K. Ream wrote:
​I have just created PR #4767. It requires mypy's strict_optional setting for leoNodes.py.
...
Aha: It's fine to allow x: whatever | None = None in method arguments. mypy will check to make sure that the method's code never calls x.y where x might be None. Experience shows casts (or minor local reorgs) are all that are usually needed to pass mypy's muster. In short, type widening is harmless in method arguments.

Otoh, type narrowing is excellent in method returns. 

This is in essence Postel's law (see Robustness Law) - be strict in what you emit, be generous in what you accept.

Edward K. Ream

unread,
Jul 1, 2026, 12:09:54 PM (2 days ago) Jul 1
to leo-e...@googlegroups.com
Nice.

And Aha/Doh: Because Leo has traditionally used strict_optional = False, almost all of Leo's return annotations already look strict.

This means that mypy will complain with strict_option = True only if the return annotations must actually be '-> x | None' instead of '-> x'.

Oh Joy. As a result of the changes to c.rootPosition and c.currentPosition, many of the apparently strict '-> Position' legacy return annotations should actually become valid. Do you see why? This is way cool.

Edward
Reply all
Reply to author
Forward
0 new messages