This repository was archived by the owner on Aug 21, 2025. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 61
feat: improve typing definitions, introduce mypy in CI #338
Merged
Conversation
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
introduce flake.nix configuration leveraging the already existing information in pyproject.toml, by using pyproject.nix to parse it and generate the correct derivation. this way, any updates to pyproject.toml should automatically be reflected in the flake configuration, with low additional maintaining overhead required. the only time when this may come as a problem is due to dependabot constantly upgrading the packages to versions that do not exist yet in nixpkgs. it may be required to force dependabot to also update the nixpkgs version declared in the flake for it to not also break (or at least to upgrade less often)
… pre-commit by adding `dependency-groups`, `pyproject.nix` can now parse the dependency groups and add these to the project, by passing `groups = ["dev"]` when calling the renderer. special care was needed in order to expose `ruff` and `pre-commit` to the toplevel of the environment. by default, it will treat them as python dependencies and hide them in the python wrapper. by using `groupBy`, we split the dependency list in "python" dependencies and top level ones, such that they can be exposed. sadly, these toplevel packages are not exposed in `python.pkgs` so we must manually override them in a case by case basis.
ensure that `nix develop` works on CI
…p action do not rely on poetry to run tests, we'd like to not rely on it from within the nix environment
use manual `supabase start` command instead
for some reason, `supabase start` does not seem to work on `macos-latest` host in CI
it seems that pyright is better and faster than mypy in most if not all cases, and the only reason I wasn't using it before is because it was not working with eglot's default config.
this makes it available to all users, preparing for including it into the standard CI later on
there's still a lot of issues that I intend to improve upon. this is just an initial set of changes to ensure that `mypy` doesn't complain. we should further strengthen the mypy checking rules (currently there are a lot of `Any`s in the code) as the code improves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Improves type safety by refining typing definitions across the realtime module and integrates MyPy into CI.
- Updated typing annotations (e.g.,
Mapping,TypedDict, callback generics) and reducedAnyusage. - Converted
Messageto a PydanticBaseModeland removed the brokensummarymethod. - Added MyPy configuration and CI steps for automated type checking.
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| realtime/types.py | Refined enum type hint, updated callback and TypedDict definitions |
| realtime/message.py | Switched Message to Pydantic BaseModel and adjusted payload type |
| realtime/exceptions.py | Broadened error message parameter type |
| realtime/_async/push.py | Switched to Pydantic Message, updated payload types and hook logic |
| realtime/_async/presence.py | Replaced callback dict with individual attributes and updated hook flows |
| realtime/_async/client.py | Fixed send/receive for Message, updated connection typing and logging |
| realtime/_async/channel.py | Updated callback signatures, replaced raw dicts with Message, removed summary |
| pyproject.toml | Added pydantic dependency and MyPy settings |
| .github/workflows/ci.yml | Added MyPy type-check steps using Poetry and Nix |
Comments suppressed due to low confidence (3)
realtime/_async/channel.py:318
- [nitpick] Parameter
typeshadows the built-intypeandfiltershadows the built-infilter. Consider renaming to more descriptive names likeevent_typeandfilter_argsto improve readability and avoid confusion.
def _on(
.github/workflows/ci.yml:39
- [nitpick] There are duplicate
Type checksteps in the CI workflow (one using Poetry and one using Nix). Consider consolidating them or giving each a distinct name to clarify their purpose.
- name: Type check
realtime/_async/channel.py:265
- Changed the payload for the
phx_closeevent from the original"leave"string to an empty dict, which may break consumers expecting a termination reason. Consider restoring the original payload or updating callers accordingly.
self._trigger(ChannelEvents.close, {})
| @dataclass | ||
| class Message: | ||
|
|
||
| class Message(BaseModel): |
Copilot
AI
Jul 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removal of the __hash__ method makes Message instances unhashable; if these objects need to be used in sets or dict keys, consider reintroducing a custom __hash__ implementation or documenting this change.
grdsdev
suggested changes
Jul 10, 2025
also enforce that mypy uses python 3.9 only rules.
Pull Request Test Coverage Report for Build 16199329439Details
💛 - Coveralls |
grdsdev
approved these changes
Jul 10, 2025
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
What kind of change does this PR introduce?
Improves typing definitions enough for
mypywith a weak rule set to accept them. This is mostly an initial patch set to ease reviewing, as there are many things to improve still - mainly, removeAnyusage in favor of generics, and improve the types of payloads with actualpydantic-based validation, with the ultimate goal of making the code type check withstrict = true.It also adds a new step to enforce
mypy ./realtimein CI from now on.What is the current behavior?
No type checking at all.
What is the new behavior?
Additional context
This should contain no breaking changes, just some rewiring of internal plumbing. The only real change I made is removing the
AsyncRealtimeClient.summarymethod, which was broken as it used a non existantchannel.listenersproperty.