Improve public API type annotations & fix unit test type errors #248

h4l · 2025-03-25T17:33:29Z

Hey, thanks for the Python cloudevents library, I've been using it (via functions_framework). In working with it I've noticed a few ways the types of your API could be tweaked to make things smoother for users. Also, while looking into the code, I noticed there were quite a few type errors in the test modules when running mypy, and mypy wasn't being run in CI to enforce the types. I figured I could help with that while offering some tweaks to the public API types.

Changes

This PR:

Fixes the existing type errors in tests
- There's more that could be done to increase type coverage in tests (e.g. annotating more fixture arguments) but I've mostly fixed the type errors that were actively being reported
Improves typing of some public APIs:
- @overloads for from_binary and from_structured to correctly type the return when the event_type arg is None
- Support the chained/self-returning Set*() methods on cloudevents.sdk.event.v1.Event (previously you couldn't chain more than one call without type errors, as the return type did not have Set* methods)
- Support passing non-dict Header classes to the from_http() conversion functions. They were requiring real dict types, so passing a headers object from an http request object would be a type error.
- Functions taking dicts as inputs (such as from_dict()) are now typed to take Mapping rather than the exact dict type, to allow passing dict-like objects, as well as read-only dicts.
Runs mypy in CI via tox, both for the cloudevents module, and the two samples
Checks the types of the two samples directories:
- The above headers change fixes a type error in one sample
- Also fixed a trivial mypy complaint due to not annotating -> None: function returns.
Fixes an unrelated CI test failure on Python 3.8, due to Python 3.8 now being out of support, and the sanic test dependency having dropped support for 3.8 in its latest release

One line description for the changelog

Improve public API type annotations, fix type errors in tests/examples and run type checks in CI
Tests pass
Appropriate changes to README are included in PR

cloudevents/tests/test_pydantic_events.py

cloudevents/tests/test_http_events.py

cloudevents/tests/test_event_pipeline.py

cloudevents/tests/test_event_from_request_converter.py

cloudevents/sdk/event/v1.py

kafka.conversion.from_binary() and from_structured() return AnyCloudEvent type var according to their event_type argument, but when event_type is None, type checkers cannot infer the return type. We now use an overload to declare that the return type is http.CloudEvent when event_type is None. Previously users had to explicitly annotate this type when calling without event_type. This happens quite a lot in this repo's test_kafka_conversions.py — this fixes quite a few type errors like: > error: Need type annotation for "result" [var-annotated] Signed-off-by: Hal Blackburn <hwtb2@cam.ac.uk>

The v1.Event self-returning Set*() methods like SetData() were returning BaseEvent, which doesn't declare the same Set* methods. As a result, chaining more than one Set* method would make the return type unknown. This was causing type errors in test_event_pipeline.py. The Set*() methods now return the Self type. Signed-off-by: Hal Blackburn <hwtb2@cam.ac.uk>

mypy was failing with lots of type errors in test modules. I've not annotated all fixtures, mostly fixed existing type errors. Signed-off-by: Hal Blackburn <hwtb2@cam.ac.uk>

from_http() conversion function was requiring its headers argument to be a typing.Dict, which makes it incompatible with headers types of http libraries, which support features like multiple values per key. typing.Mapping and even _typeshed.SupportsItems do not cover these types. For example, samples/http-image-cloudevents/image_sample_server.py was failing to type check where it calls `from_http(request.headers, ...)`. To support these kind of headers types in from_http(), we now define our own SupportsDuplicateItems protocol, which is broader than _typeshed.SupportsItems. I've only applied this to from_http(), as typing.Mapping is OK for most other methods that accept dict-like objects, and using this more lenient interface everywhere would impose restrictions on our implementation, even though it might be more flexible for users. Signed-off-by: Hal Blackburn <hwtb2@cam.ac.uk>

Tox now runs mypy on cloudevents itself, and the samples. Signed-off-by: Hal Blackburn <hwtb2@cam.ac.uk>

Signed-off-by: Hal Blackburn <hwtb2@cam.ac.uk>

Mapping imposes less restrictions on callers, because it's read-only and allows non-dict types to be passed without copying them as dict(), or passing dict-like values and ignoring the resulting type error. Signed-off-by: Hal Blackburn <hwtb2@cam.ac.uk>

Tests were failing because the sanic dependency dropped support for py3.8 in its current release. sanic is now pinned to the last compatible version for py3.8 only. Signed-off-by: Hal Blackburn <hwtb2@cam.ac.uk>

h4l · 2025-04-01T14:03:55Z

Thanks for running CI, I see mypy failed, I should be able to fix it up later on.

xSAVIKx · 2025-04-01T14:47:07Z

Thank you for the PR. I haven't got the time to review it yet, but hope to allocate some soon.

Pydantic added by_alias and by_name keyword arguments to BaseModel.model_validate_json in 2.11.1: pydantic/pydantic@acb0f10 This caused mypy to report that that the Pydantic v2 CloudEvent did not override model_validate_json() correctly. Our override now accepts these newly-added arguments. They have no effect, as the implementation does not use Pydantic to validate the JSON, but we also don't use field aliases, so the only effect they could have in the superclass would be to raise an error if they're both False. Signed-off-by: Hal Blackburn <hwtb2@cam.ac.uk>

h4l · 2025-04-01T18:20:47Z

Cool, that's no problem.

CI should pass now (passes in my fork). There's a chance that a dependency will update and drop Python 3.8 support as it's out of support, which could cause mypy to fail. Currently mypy.ini has python_version = 3.8, we could bump this to 3.9 so that libraries that start using syntax that 3.8 doesn't support don't cause mypy to fail. I didn't want to change that unilaterally though!

xSAVIKx

@h4l overall LGTM. I'm hesitant regarding the SupportsDuplicateItems usage while it breaks the existing typing APIs and will be a breaking change for the library users. We need to make it work without narrowing down the typings

h4l · 2025-05-19T13:00:18Z

Thanks for looking at this. I'll take another look at the changes around SupportsDuplicateItems and make sure it's not narrowing the previous API types.

Although our types.SupportsDuplicateItems type is wider than Dict and Mapping, it's not a familar type to users, so explicitly accepting Mapping in the from_http() functions should make it more clear to users that a dict-like object is required for the headers argument. Signed-off-by: Hal Blackburn <hwtb2@cam.ac.uk>

h4l · 2025-05-21T14:59:28Z

@xSAVIKx I just had a look into this and pushed an update.

The SupportsDuplicateItems type I added originally is actually wider than the previous Dict/Mapping type the from_http() functions accepted, so it was widening rather than narrowing the type. (The SupportsDuplicateItems type only requires a .items() method). However I can see that only mentioning this type in the signature is unintuitive, as it's not a standard type, so I've changed the signature to accept Mapping or SupportsDuplicateItems, which should make it clearer that it accepts a dict-like object.

I needed to widen the type originally because HTTP header types from libraries like requests do not satisfy Dict/Mapping, so using these types on from_http() was not sufficient.

Does that help with your concern? If not perhaps I've misunderstood you.

I also needed to stop running CI/mypy against Python 3.8 because it's now well out of support, and multiple dependencies are shipping versions that mypy fails on with 3.8 compatibility. I've also added 3.12 and 3.13 to the the CI matrix.

8000

xSAVIKx

@h4l , you had the 3.8 support prior to the latest change I believe, right? I'd rather keep it if possible for this change and then remove support for it in a separate PR. I understand it requires outdated dependencies which is fine imo. Otherwise the typings we're using I believe all were already supported by 3.8, right?

h4l · 2025-05-21T16:21:40Z

@xSAVIKx yes, 3.8 was working when I opened the PR. However because the repo has un-pinned dependencies in requirements.txt without a lockfile, some deps have new versions now that no longer work with 3.8. We can keep 3.8 but it will mean adding extra rules to requirements.txt to pin deps to old versions. I could do that if you prefer though. (Type checking was failing because of this when I made this change today.)

xSAVIKx · 2025-05-21T20:25:11Z

I'd say pinning versions with Python version checks makes sense. And those were test dependencies only, right?

xSAVIKx · 2025-05-21T20:27:37Z

Thanks a lot for your efforts @h4l 🙏

Python 3.8 is unsupported and dependencies (such as pydantic) are now shipping releases that fail to type check with mypy running in 3.8 compatibility mode. We run mypy in py 3.8 compatibility mode, so the mypy tox environments must only use deps that support 3.8. And unit tests run by py 3.8 must only use deps that support 3.8. To constrain the deps for 3.8 support, we use two constraint files, one for general environments that only constrains the dependencies that python 3.8 interpreters use, and another for mypy that constraints the dependencies that all interpreters use. Signed-off-by: Hal Blackburn <hwtb2@cam.ac.uk>

h4l · 2025-05-23T12:12:35Z

@xSAVIKx no problem!

I'd say pinning versions with Python version checks makes sense. And those were test dependencies only, right?

There are two deps causing trouble at the moment, sanic which is a test dep and pydantic which is runtime. We only need to constrain them when running mypy and pytest though, so end users aren't affected. What I've done is to add two pip constraint.txt files that limit the versions of these two deps:

For mypy type checks, we need to constrain the dep versions for any interpreter, because mypy itself emulates python 3.8 behaviour.
For pytest, only the tests running with python 3.8 need constraining