Uh oh!
There was an error while loading. Please reload this page.
Type-check the whole faust package and delete the mypy ratchet - #758
Merged
Conversation
PEP 484 dropped implicit Optional and mypy has defaulted to `no_implicit_optional=True` since 0.990, so every `x: T = None` parameter in the package was an error. That single pattern accounted for the largest share of the backlog the ratchet in pyproject.toml is silencing -- 269 of the 512 errors mypy reports with the ratchet disabled, spread over 62 files. Rewrite them as `x: Optional[T] = None` with hauntsaninja's `no_implicit_optional` codemod, then re-run `scripts/lint` so isort and black normalise the imports it added. The transformation is annotation-only: it widens each parameter's declared type to include the `None` that the default already allowed, so no call that type-checked before stops doing so, and no runtime behaviour changes. `scripts/check` and the unit and functional suites all still pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7
These are the protocol definitions the rest of the package implements, so fixing them first keeps the implementation modules from being annotated against signatures that are themselves wrong. `Param.__get__` returned `OT` unconditionally and needed a `# type: ignore` for the `obj is None` branch that hands back the descriptor. Declare the two cases as overloads instead, mirroring the split `property` already declares, and drop the ignore. `Param.prepare_set`, `prepare_init_default`, `on_set` and `validate_after` all claimed to take and return a non-optional value, but every one of them handles `None` explicitly -- `prepare_set` returns it unchanged when `allow_none` is set. Widen them to `Optional` so the declared contract matches what the code actually does. `StreamT.__next__` returned a bare `T`, a type variable that appears nowhere else in the class; `StreamT` is generic in `T_co`. It was meaningless as written, so bind it to `T_co`, which is covariant and so valid in return position. `AppT.conf`'s getter was a `...`-bodied property that mypy read as a concrete method returning nothing. Mark it abstract, which is what it always was -- `App` provides the concrete property. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@## master #758 +/- ##
==========================================
+ Coverage 96.06% 96.07% +0.01%
==========================================
Files 103 103 Lines 11072 11081 +9 Branches 1191 1189 -2 ==========================================
+ Hits 10636 10646 +10 + Misses 345 343 -2 - Partials 91 92 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`mypy -p faust` now checks all 164 modules in the package with nothing silenced wholesale, so the `ignore_errors` ratchet in pyproject.toml is gone -- all 78 entries, not a subset. The previous commit cleared the implicit-Optional backlog; this one fixes what was left. The bulk is honest annotation work: attributes declared on the class that were only ever assigned in `__init__`, `Optional` added where a value really can be None, protocol signatures corrected to match every implementer, and generic bases parameterised (`Schema(SchemaT[KT, VT])`, `TopicBuffer(Iterator[Tuple[TP, Any]])`) so their element types survive to callers. The change is annotation-only by construction: no runtime behaviour differs from the previous commit. Where the checker was pointing at a genuine defect, the code is left exactly as it was and the error is silenced with a specific `# type: ignore[code]` whose comment says, prefixed `XXX`, what is actually broken -- so the checker stops flagging it without the diff quietly pretending it is fine. Those XXX markers are the useful residue of this pass; each one is a real bug worth its own fix and test: - aiokafka.py `ProducerThread._shutdown_thread` is a sync override of mode's `async def`, which `_serve()` awaits in a `finally:` -- so shutdown hits `await None` and raises TypeError. - aiokafka.py calls `Topic._on_published` without its required positional `fut`, so the wait-branch raises TypeError. - confluent.py `Producer.key_partition` reaches for `list_topics` on the Faust producer rather than the confluent one, so it is dead on arrival. - tables/base.py keys `_partition_timestamp_keys` lookups with the whole `(start, end)` window range where the map is keyed on `end`, so `on_window_close` never receives the aggregated window data. - recovery.py applies a changelog event for an untracked TP using the previous iteration's `table`/`offsets`/`bufsize` locals. - auth.py `SSLCredentials()` defaults `purpose` to None, which `ssl.create_default_context` rejects with TypeError. - cli/base.py `AppCommand.__init__` dereferences `self.app.conf` unconditionally, so the documented `require_app = False` escape hatch raises AttributeError. - `faust.version_info` is built from regex string groups, so `.major` is the literal 'v'. `faust.types.settings.settings` keeps a narrow, documented `disable_error_code = ["empty-body"]`: every setting there is a docstring-only method consumed by the `@sections.<Section>.setting(...)` decorator, which discards the function and returns a descriptor, so the body is never executed. That is one error code, not the module -- every other error in the file is still reported. `scripts/check` (isort, black, flake8, mypy) passes with the pinned toolchain from requirements/test.txt, and the unit and functional suites are unchanged at 2207 passed, 4 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7
The lint job failed on the previous commit with three `str-bytes-safe`
errors in faust/stores/aerospike.py that do not reproduce under Python
3.11.
Each was an implicitly-concatenated f-string whose `# type: ignore` sat on
the first fragment while the `{key}` interpolation that actually trips the
check sat on the second:
self.log.error(
f"FaustAerospikeException Error in set for " # type: ignore[str-bytes-safe]
f"table {self.table_name} exception {ex} key {key}"
)
PEP 701 changed f-string tokenisation in 3.12, so mypy attributes the error
to a different fragment depending on the interpreter it runs under -- the
first fragment on 3.11, the second on 3.12. `python_version = "3.12"` in
pyproject.toml pins the typeshed target but not the parser, so a contributor
on 3.11 sees a clean tree while the lint job (PYTHON_LATEST = 3.12) does not.
Join each of the three into a single physical line so the interpolation and
the ignore cannot land on different lines under any interpreter. The
concatenated message text is unchanged, so the log output is identical. A
scan of the package confirms no other `# type: ignore` sits on an implicitly
concatenated string.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7mode ships PEP 561 type information, so its annotations are part of faust's own type-check surface rather than an opaque `Any`. That cuts both ways: it is why mypy can see through into `Service`/`ServiceThread` at all, and it is why the result depends on which mode a contributor happens to have installed. Measured on this tree with the pinned mypy 2.3.0: mode-streaming 0.4.1 -> 38 errors in 14 files mode-streaming 0.6.0 -> 0 errors The difference is annotation precision in mode, not anything faust does. 0.4.1 declares `want_seconds(float)` where the argument is really `Seconds` (`timedelta | float | str`) and `level_name(int)` where it is `str | int`, and its generic containers infer less, so `channel_it` and friends need explicit annotations that 0.6.0 makes unnecessary. `requirements.txt` said `>=0.4.0`, so a contributor resolving an older mode saw 38 errors on a tree the lint job calls clean -- exactly the reproducibility problem the repo already avoids by pinning mypy in typecheck.txt. Raise the floor to the version CI has been resolving and testing against all along. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7
This was referenced Aug 6, 2026
Uh oh!
There was an error while loading. Please reload this page.
wbarnha pushed a commit
that referenced
this pull request
Aug 6, 2026
Both were found by the type checker in #758 and left marked `XXX` there because fixing them changes runtime behaviour. `ThreadedProducer._shutdown_thread` was a plain `def` overriding `mode.threads.ServiceThread._shutdown_thread`, which is `async def` and is awaited by `_serve()` in a `finally:`. Every shutdown of the producer thread therefore evaluated `await None` and raised TypeError. The thread only recovered because `_start_thread` catches that exception and calls `set_shutdown()` before re-raising -- so mode's teardown (`on_thread_stop`, stopping children, futures and exit stacks) never ran, and the thread died with a traceback instead of stopping cleanly. The override also scheduled `on_thread_stop()` with `asyncio.run_coroutine_threadsafe` onto `self.thread_loop` -- the loop that was about to stop, and the loop already running `_serve()`. Because the TypeError tore down `run_until_complete` immediately, that coroutine never got a chance to run, so the producer was never flushed or stopped on this path. Make it `async def` and await `super()._shutdown_thread()`, which runs `on_thread_stop()` and the rest of mode's teardown in order. The once-only guard is kept; when shutdown has already been initiated the shutdown event is still set, matching what the old TypeError path ended up doing via `_start_thread`. `ThreadedProducer.publish_message(wait=True)` called `fut.message.channel._on_published(message=..., state=..., producer=...)`. `Topic._on_published` takes the send future as a required *positional* `fut` and reads the result off it, so the call raised `TypeError: Topic._on_published() missing 1 required positional argument`. The waiting branch has no such future -- `send_and_wait` has already resolved -- so complete the message directly instead: report the sensor, set the result, and invoke the callback, which is what `Topic.publish_message(wait=True)` does via `_finalize_message`. The non-waiting branch keeps using `_on_published` as a done-callback, where `add_done_callback` supplies `fut`. `test_publish_message_with_wait` did not catch this because its channel is a bare `Mock`, which accepts any call; the new test uses a real topic and fails with the TypeError above against the previous code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7
wbarnha pushed a commit
that referenced
this pull request
Aug 6, 2026
`SSLCredentials.__init__` defaulted `purpose` to None and passed it straight into `ssl.create_default_context(purpose=...)`, which begins if not isinstance(purpose, _ASN1Object): raise TypeError(purpose) So `SSLCredentials()` and `SSLCredentials(cafile=...)` -- any call that does not supply an explicit `context` -- raised `TypeError: None`. The class could only ever be constructed by handing it a context built elsewhere, which defeats the cafile/capath/cadata parameters entirely. Default to `ssl.Purpose.SERVER_AUTH`: the same default `create_default_context()` itself applies, and the correct one for a client verifying a broker. It implies `check_hostname=True` and `verify_mode=CERT_REQUIRED`, which the tests assert. An explicitly passed `purpose` is still forwarded unchanged. Found by the type checker in #758 and marked `XXX` there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7
wbarnha pushed a commit
that referenced
this pull request
Aug 6, 2026
`VersionInfo` declares `major`, `minor` and `micro` as ints, but the module
splatted the regex groups `(prefix, version, suffix)` into them positionally:
VersionInfo(major=None, minor='0.11.5', micro='')
So `faust.version_info.major` was the `'v'` prefix or None, `.minor` was the
whole version string, `.micro` was the suffix, and `.releaselevel` was always
None -- every field wrong except by accident.
Parse the dotted numbers properly instead, putting any non-numeric tail
(`dev1+g1234`, `rc1`, a local segment) into `releaselevel`. Missing components
pad with zero, and an unparsable version degrades to `VersionInfo(0, 0, 0, ...)`
rather than raising, so `import faust` can never fail on the version string --
the old `RuntimeError('THIS IS A BROKEN RELEASE!')` branch is gone with it.
This changes a public value, which is why #758 left it marked `XXX` rather than
fixing it: code reading `faust.version_info.minor` as the version *string* must
switch to `faust.__version__`, which is unchanged. Nothing in the repo consumes
it and no docs reference it.
`_parse_version` is injected into the lazy module's `__dict__` so it is
reachable for testing; it is private and stays out of `__all__` and `dir()`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7wbarnha pushed a commit
that referenced
this pull request
Aug 6, 2026
`Collection._del_old_keys` read `_partition_timestamp_keys` with self._partition_timestamp_keys.get((partition, window_range)) where `window_range` is the whole `(start, end)` tuple. The map is keyed `(partition, range_end)` -- an `(int, float)` pair -- written that way by `_maybe_set_key_ttl` and read that way by `_maybe_del_key_ttl`. So the lookup could never hit. `triggered_windows` was always `[None, ...]`, `window_data` stayed empty, and `on_window_close` was only ever handed the raw per-key value instead of the aggregated window data it exists to receive. Use `(partition, window_range[1])`, matching the writer. This is user-visible: applications with an `on_window_close` handler will start receiving the aggregated data the API always promised. That is why #758 marked it `XXX` instead of fixing it. Two existing tests relied on `mock_ranges` returning bare floats, which is not what `_window_ranges` yields; they now pass real `(start, end)` tuples. Their assertions are unchanged and the ranges still match nothing in `_partition_timestamp_keys`, so they continue to cover the untriggered path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7
wbarnha pushed a commit
that referenced
this pull request
Aug 6, 2026
Two bugs in `Recovery`, both found by the type checker in #758. `_slurp_changelogs` classifies each event's TP as active or standby and binds `table`, `offsets` and `bufsize` accordingly. The `else:` branch for a TP that is neither only logged `"recovery unknown topic"` and fell through -- so the event was applied using the *previous* iteration's bindings: written into an unrelated table's buffer and offset map, and passed to that table's `on_changelog_event`. On the first event of the loop there is nothing bound yet, so it raised `UnboundLocalError` instead. Skip applying an event for an untracked TP. Note a bare `continue` would be wrong: the statements at the bottom of the loop body -- `_maybe_signal_recovery_end()` and the standby-ready bookkeeping -- must keep running on every iteration, or recovery-end signalling loses a trigger. Only the event-application block is skipped. `detect_aborted_tx` compared `await self.app.consumer.position(tp) >= highwater` unguarded. `ConsumerT.position` is `Optional[int]` and does return None when a partition has no position yet, so that raised TypeError -- swallowed by the caller's `except Exception`, which then silently skipped the aborted-transaction fixup for every remaining partition in the loop. Skip a TP with no position. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7
wbarnha pushed a commit
that referenced
this pull request
Aug 6, 2026
`_app_from_str` returns None for a `require_app = False` command invoked without `-A` -- that is the documented escape hatch, and `faust completion` is exactly such a command. `_finalize_app` handed that None straight back, and `AppCommand.__init__` then did self.key_serializer = key_serializer or self.app.conf.key_serializer unconditionally, so the command died with `AttributeError: 'NoneType' object has no attribute 'conf'`. The escape hatch was unusable: `faust completion` could not run without the `-A` it is written not to need. Make `AppCommand` tolerate having no app. `self.app` becomes a property over an `Optional[AppT]`, the serializer defaults fall back to None when there is no app, and `on_stop` and `blocking_timeout` no longer assume one. Behaviour with an app present is unchanged, and a command with `require_app = True` still gets the same `UsageError` from `_app_from_str` as before. Found by the type checker in #758 and marked `XXX` there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7
wbarnha pushed a commit
that referenced
this pull request
Aug 6, 2026
`Producer.key_partition` called `self._producer_thread.producer.list_topics()`. `ProducerThread.producer` is the *Faust* Producer; the confluent_kafka handle is `ProducerThread._producer`. Faust producers have no `list_topics`, so the method raised AttributeError and was dead on arrival. Read the confluent producer instead. `Consumer.verify_event_path` delegated to `self._thread.verify_event_path(...)`, but neither `ConsumerThread` nor `ConfluentConsumerThread` defines it, so the commit-livelock detector (`_commit_livelock_detector` -> `verify_all_partitions_active`) raised AttributeError on every tick. Add the no-op to `ConfluentConsumerThread`, matching the documented no-op stub the base `faust.transport.consumer.Consumer.verify_event_path` already is. This makes livelock detection inert for this driver rather than raising -- a real implementation is separate work. Both were found by the type checker in #758 and marked `XXX` there. Note these tests do not run here or in CI: tests/unit/transport/drivers/test_confluent.py starts with `pytest.importorskip("confluent_kafka")`, and confluent-kafka is the optional `faust[ckafka]` extra, which the CI test environment does not install. The fixes are verified by reading the class definitions, not by an executed test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7
wbarnha pushed a commit
that referenced
this pull request
Aug 6, 2026
`mypy -p faust` now checks the whole package (#758 removed the ratchet), and this branch fails it: faust/events.py:174: error: Incompatible default for parameter "timestamp" (default has type "object", parameter has type "float | None") [assignment] The three older sentinels get away with a bare `object()` because the parameters they default are annotated loosely -- `key: K` and `value: V` resolve through `Any`, and `headers: Any` is `Any` outright. `timestamp` is `Optional[float]`, which `object()` does not inhabit. Cast the sentinel to `float` rather than widening the parameter: the annotation callers see stays `Optional[float]`, which is the useful one, and `typing.cast` is a no-op at runtime, so `USE_EXISTING_TIMESTAMP` is still a unique object and the `is` comparison in `forward()` is unchanged. Verified: merged into master, `mypy -p faust` reports "Success: no issues found in 164 source files"; tests/unit/test_events.py and tests/functional/test_streams.py pass (61 passed). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7
wbarnha pushed a commit
that referenced
this pull request
Aug 6, 2026
`mypy -p faust` now checks the whole package (#758 removed the ratchet), and this branch fails it with 8 errors -- two implicit-Optional parameters, each producing one `[assignment]` plus one `[override]` against every supertype that declares it: faust/sensors/otel.py:320: error: Argument 5 of "on_stream_event_out" is incompatible with supertype "faust.sensors.monitor.Monitor"; supertype defines the argument type as "dict[Any, Any] | None" [override] ... and the same against Sensor and SensorInterfaceT faust/sensors/otel.py:320: error: Incompatible default for parameter "state" (default has type "None", parameter has type "dict[Any, Any]") [assignment] ... and the same four for "view" at :441 PEP 484's implicit-Optional was removed in mypy 0.990, so `state: Dict = None` no longer means `Optional[Dict]`; it means `Dict`, which both rejects the `None` default and narrows the parameter relative to the base class. The bodies already handle `None` (`if state is not None:`) and the base signatures already say `Optional`, so spelling it out is the correction, not a behaviour change. Verified: merged into master, `mypy -p faust` reports "Success: no issues found in 165 source files"; tests/unit/sensors/test_otel.py passes (18 passed, with opentelemetry installed -- it importorskips otherwise). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7
wbarnha added a commit
that referenced
this pull request
Aug 10, 2026
* Add OpenTelemetry metrics monitor (faust[opentelemetry]) Add OpenTelemetryMonitor, a sensor that reports the same metric set as the Statsd and Datadog monitors through the OpenTelemetry metrics API, so Faust metrics can be exported to any OpenTelemetry-compatible backend (OTLP, Prometheus, console, ...). Faust depends only on opentelemetry-api via the new optional faust[opentelemetry] extra; every instrument is a cheap no-op until the application configures a global MeterProvider. Following OpenTelemetry conventions, each instrument is dimensioned by attributes (topic, partition, stream, table, ...) rather than baking those into the metric name as Statsd does. - faust/sensors/otel.py: OTelMetrics instrument container + OpenTelemetryMonitor. - requirements/extras/opentelemetry.txt + setup.py bundle + test requirements. - tests/unit/sensors/test_otel.py: 17 tests driving the hooks against an in-memory SDK metric reader. - docs: user-guide section, installation bundle entry, reference autodoc page. Assisted-by: Claude Opus 4.8 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LJ1voGxC8Nqs3AUPKnFVzZ * test(otel): cover injected OTelMetrics container path Adds a test that passes a pre-built OTelMetrics container via the `metrics=` kwarg, covering the last uncovered line/branch in OpenTelemetryMonitor.metrics (patch coverage 98.9% -> 100%). Assisted-by: Claude Opus 4.8 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LJ1voGxC8Nqs3AUPKnFVzZ * Make the two implicit-Optional parameters in otel.py explicit `mypy -p faust` now checks the whole package (#758 removed the ratchet), and this branch fails it with 8 errors -- two implicit-Optional parameters, each producing one `[assignment]` plus one `[override]` against every supertype that declares it: faust/sensors/otel.py:320: error: Argument 5 of "on_stream_event_out" is incompatible with supertype "faust.sensors.monitor.Monitor"; supertype defines the argument type as "dict[Any, Any] | None" [override] ... and the same against Sensor and SensorInterfaceT faust/sensors/otel.py:320: error: Incompatible default for parameter "state" (default has type "None", parameter has type "dict[Any, Any]") [assignment] ... and the same four for "view" at :441 PEP 484's implicit-Optional was removed in mypy 0.990, so `state: Dict = None` no longer means `Optional[Dict]`; it means `Dict`, which both rejects the `None` default and narrows the parameter relative to the base class. The bodies already handle `None` (`if state is not None:`) and the base signatures already say `Optional`, so spelling it out is the correction, not a behaviour change. Verified: merged into master, `mypy -p faust` reports "Success: no issues found in 165 source files"; tests/unit/sensors/test_otel.py passes (18 passed, with opentelemetry installed -- it importorskips otherwise). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7 * Fix the two type errors that only appear once opentelemetry is installed This PR adds `-r extras/opentelemetry.txt` to `requirements/test.txt`, so the lint job -- which installs test.txt and runs `mypy -p faust` via `scripts/check` -- will have opentelemetry present. That matters: with `ignore_missing_imports = true`, an absent package resolves to `Any` and nothing in it can be wrong. Once it is installed mypy checks against the real API and reports: faust/sensors/otel.py:44: error: Incompatible types in assignment (expression has type "None", variable has type Module) [assignment] faust/sensors/otel.py:47: error: Module "opentelemetry.metrics" has no attribute "Gauge"; maybe "_Gauge"? [attr-defined] The second one is a real bug, not a checker artifact. `opentelemetry.metrics` has never exported a public `Gauge`: the synchronous gauge landed in opentelemetry-api 1.23.0 -- the floor `extras/opentelemetry.txt` pins -- as `_Gauge`, and `__all__` still lists it that way in 1.44.0. The class is *named* `Gauge` (which is why `Meter.create_gauge` renders as returning `opentelemetry.metrics.Gauge`), but the importable name is the underscore one. So `from opentelemetry.metrics import Gauge` would fail outright were it not sitting under `if typing.TYPE_CHECKING`. Import `_Gauge as Gauge` instead, and say why in a comment so it does not look like an accident. The first is the module-as-sentinel pattern `sensors/prometheus.py` already carries: `otel_metrics` is both the module and the "is the extra installed?" flag that `OpenTelemetryMonitor.meter` checks, and mypy binds the name to the module type at the import. Same ignore, same explanation, as prometheus.py. Verified against master with opentelemetry-api 1.44.0 installed: `mypy -p faust` reports "Success: no issues found in 165 source files". tests/unit/sensors/test_otel.py passes (18 passed); it importorskips when the extra is absent, which is why nothing caught this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012N2yysiNbzzVcvYhtrpsM7 --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mypy -p faustnow checks all 164 modules in the package with nothing silenced wholesale, so theignore_errorsratchet inpyproject.tomlis gone — all 78 entries, not a subset.Measured with the pinned mypy 2.3.0 from
requirements/typecheck.txt, in an environment matching the lint job (the installed dependency set changes what mypy reports, since absent packages becomeAny):master(4af976b)OptionalcodemodThe commits
Optionaland mypy has defaulted tono_implicit_optional=Truesince 0.990, so everyx: T = Noneparameter was an error. That single pattern was 269 of the 512, across 62 files. Rewritten with hauntsaninja'sno_implicit_optionalcodemod, thenscripts/lintto normalise the imports it added.faust/types— the protocol definitions the rest of the package implements, fixed first so implementations are not annotated against signatures that are themselves wrong.python_version = "3.12"pins the typeshed target but not the parser, so three ignores that looked correct on 3.11 missed their line on the lint job. Joined into single physical lines so the interpolation and the ignore cannot separate.mode-streaming>=0.6.0— see below.Annotation-only
No runtime behaviour differs from
master. Where mypy was pointing at a genuine defect, the code is left exactly as it was and the error is silenced with a specific# type: ignore[code]whose comment says — prefixedXXX— what is actually broken, so the checker stops flagging it without the diff quietly pretending it is fine.Those
XXXmarkers are the useful residue of this pass. Each is a real bug that deserves its own fix and test, and none is addressed here:aiokafka.py—ProducerThread._shutdown_threadis a sync override of mode'sasync def, which_serve()awaits in afinally:. Shutdown therefore evaluatesawait Noneand raisesTypeError.aiokafka.py—Topic._on_publishedis called without its required positionalfut, so the wait-branch raisesTypeError.confluent.py—Producer.key_partitionreaches forlist_topicson the Faust producer rather than the confluent one, so it is dead on arrival.tables/base.py—_partition_timestamp_keysis looked up with the whole(start, end)window range where the map is keyed onend, soon_window_closenever receives the aggregated window data.recovery.py— a changelog event for an untracked TP is applied using the previous loop iteration'stable/offsets/bufsize.auth.py—SSLCredentials()defaultspurposetoNone, whichssl.create_default_contextrejects withTypeError.cli/base.py—AppCommand.__init__dereferencesself.app.confunconditionally, so the documentedrequire_app = Falseescape hatch raisesAttributeError.faust/__init__.py—version_infois built from regex string groups, so.majoris the literal'v'.Why the mode floor moved
mode ships PEP 561 type information, so its annotations are part of faust's own type-check surface rather than an opaque
Any. That is why mypy can see intoService/ServiceThreadat all — and why the result depends on which mode is installed. On this tree:mypy -p faustThe difference is annotation precision in mode, not anything faust does: 0.4.1 declares
want_seconds(float)where the argument is reallySeconds, andlevel_name(int)where it isstr | int.requirements.txtsaid>=0.4.0, so a contributor resolving an older mode saw 38 errors on a tree the lint job calls clean — the same reproducibility problem the repo already avoids by pinning mypy. The floor now matches what CI has been resolving all along.The one carve-out
faust.types.settings.settingskeeps a narrow, documenteddisable_error_code = ["empty-body"]. Every setting there is a docstring-only method consumed by the@sections.<Section>.setting(...)decorator, which discards the function and returns aParamdescriptor — the body is never executed, so the check does not apply to the pattern. That disables one error code, not the module: every other error in the file is still reported, which is a strict improvement on theignore_errorsentry it replaces.mypy stays dev/CI-only
Unchanged by this PR, but worth stating since it is the point of the ratchet:
setup.pysetsinstall_requires=reqs("requirements.txt"), and mypy lives inrequirements/typecheck.txt, reachable only viatest.txt→tests_require, which pip does not install for end users.extras_require()only globsrequirements/extras/, so it cannot leak in there either. Nothing underfaust/imports mypy.Verification
scripts/check(isort, black, flake8, mypy) passes with the pinned toolchain fromrequirements/test.txt, and the unit and functional suites are unchanged at 2207 passed, 4 skipped.On coverage:
codecov/projectpasses at +0.01% — Hits +10, Misses −2 againstmaster.codecov/patchreports 89.03%; its missing lines are lines this diff touched in code CI cannot execute — the aerospike and rocksdb drivers (optional deps not installed, unlikeconfluent.pywhich.coveragercalready omits for that reason) andcast()/type-ignore lines inside defensive branches that were already uncovered onmaster. Writing tests for defensive error paths was out of scope for an annotation-only change.🤖 Generated with Claude Code