ankitects / ankitects/anki

Enable strict_optional in mypy config

Open
#3,317 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
31.2k
Forks
3.2k
Avg merge
2d 8h
Merged PRs (30d)
72

Description

Our mypy configuration uses `no_strict_optional = true`. This tells mypy to assume nullable variables will not be none, so code like the following will pass a check, and fail at runtime instead:

```
foo: int | None = None
foo + 5
```

If we adjust the mypy config, we can get it to throw errors instead:

```diff
diff --git a/.mypy.ini b/.mypy.ini
index ef32ed7a2..1207b6aa3 100644
--- a/.mypy.ini
+++ b/.mypy.ini
@@ -22,6 +22,7 @@ exclude = (qt/bundle/PyOxidizer|pylib/anki/_vendor)

[mypy-anki.*]
disallow_untyped_defs = True
+no_strict_optional = false
[mypy-anki.importing.*]
disallow_untyped_defs = False
[mypy-anki.exporting]
```

There's quite a few in pylib that need correcting, e.g.

```
pylib/anki/importing/anki2.py:316: error: Item "None" of "Optional[DBProxy]" has no attribute "execute" [union-attr]
pylib/anki/importing/csvfile.py:114: error: Value of type "Union[str, list[str], None]" is not indexable [index]
pylib/anki/importing/csvfile.py:116: error: Value of type "Union[str, list[str], None]" is not indexable [index]
pylib/anki/importing/csvfile.py:118: error: Value of type "Union[str, list[str], None]" is not indexable [index]
pylib/anki/importing/csvfile.py:122: error: Argument 1 to "reader" has incompatible type "Union[str, list[str], None]"; expected "Iterable[str]" [arg-type]
pylib/anki/importing/apkg.py:62: error: Item "None" of "Optional[ZipFile]" has no attribute "read" [union-attr]
Found 181 errors in 24 files (checked 336 source files)
```

The first error is caused by doing 'col.db.execute()' when col.db is DBProxy | None. Adding an `assert col.db` will tell mypy that we are confident db will not be null in that case, and the error will go away. In other cases where we're not 100% sure if the value will be valid, the routine should check for null / exit early / etc.

The work on this can be done incrementally, as you can enable no_strict_optional for individual submodules at a time (e.g. pylib.importing).

Many of the errors are in legacy code (old stats, old importing code) that will eventually be removed. In such cases, sprinkling asserts above the problem lines is probably all that is required. Our goal here is not really to change the way the errors are handled at run time. The goal is to get our code working with the new setting, so we can benefit from its increased protection in other parts of the codebase.

Another upside of doing this is that pyright (the type checker used by VS Code) does not support no_strict_optional. That causes our current codebase to show a lot errors when checked with pyright.

I suspect sorting out pylib first will bring the most value. aqt would benefit from being tackled too in the future, but there are 1000+ errors at the moment. Sometimes a single assert at the top of a function will fix a bunch of errors at once, as all the subsequent code can assume some variable is non-null. So the total issues to fix are likely smaller than that. But better to start with the low-hanging fruit.

Contributor guide

Open the contributing guide

Research direction

Start with .mypy.ini and the reported errors in pylib/anki/importing/anki2.py, csvfile.py, and apkg.py. Enable strict optional handling for a small pylib module or submodule, run mypy, and resolve its nullable-value errors; done means the selected scope passes with the stricter setting without changing runtime behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
developer-experience, tooling
Issue type
Refactor
Difficulty
4/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.