Replace mypy with ty for static type checking by svij-sc · Pull Request #585 · Snapchat/GiGL

svij-sc · 2026-04-13T22:50:33Z

ty (from Astral, makers of ruff/uv) replaces mypy as the project's static type checker.

The ty fixes are introduced here: #586

Benchmarked on the code base (631 source files):

Metric	mypy	ty	Speedup
Cold cache	184.6s	0.79s	~234x
Warm cache	13.9s	0.83s	~17x

Changes:

Add ty to lint deps, remove mypy + mypy-extensions (keep mypy-protobuf for protoc stub generation)
Migrate mypy.ini config to [tool.ty.*] sections in pyproject.toml
Update Makefile type_check target: uv run ty check
Downgrade 12 rule categories to "warn" for initial migration; a follow-up PR will fix the 226 warnings and promote them back to errors

Scope of work done

Where is the documentation for this feature?: N/A

Did you add automated tests or write a test plan?

Updated Changelog.md? NO

Ready for code review?: YES

ty (from Astral, makers of ruff/uv) replaces mypy as the project's static type checker. Benchmark on this codebase (631 source files): | Metric | mypy | ty | Speedup | |-------------|----------|-------|---------| | Cold cache | 184.6s | 0.79s | ~234x | | Warm cache | 13.9s | 0.83s | ~17x | Changes: - Add ty to lint deps, remove mypy + mypy-extensions (keep mypy-protobuf for protoc stub generation) - Migrate mypy.ini config to [tool.ty.*] sections in pyproject.toml - Update Makefile type_check target: `uv run ty check` - Downgrade 12 rule categories to "warn" for initial migration; a follow-up PR will fix the 226 warnings and promote them back to errors - Update docs (CLAUDE.md, README.md, CHANGELOG.md, .claude/formatting.md) - Update .gitignore/.dockerignore cache directory references - Replace "mypy" mentions in source comments with generic "type checker" - Delete mypy.ini Co-Authored-By: shubhamvij <25601958+shubhamvij@users.noreply.github.com>

- Pin ty~=0.0.29 for reproducible builds - Add comment clarifying mypy-protobuf is for stub generation only - Exclude notebooks (*.ipynb) from type checking - Add comment about torch lacking inline types/stubs - Promote all ty rules back to default error severity (remove warn downgrades — fixes come in a follow-up PR) - Add [[tool.ty.overrides]] to suppress errors in generated *_pb2.py files that cannot be fixed Co-Authored-By: shubhamvij <25601958+shubhamvij@users.noreply.github.com>

svij-sc · 2026-04-15T01:53:48Z

+[tool.ty.src]
+exclude = ["*_pb2.py", "*_pb2.pyi", "**/*.ipynb"]
+
+[tool.ty.analysis]


Migrated from mypy.ini - removed any that could be fixed - thank you robots!

svij-sc

I updated the downstream PR where I addressed the ty fixes and to it added some new fixes identified by @kmontemayor2-sc below.

Ref: #586

kmontemayor2-sc

I do feel pretty strongly about not type ignoring torch, WDYT about including #597 as part of the follow up here?

Otherwise this LGTM, thanks for the work :)

Co-authored-by: shubhamvij <25601958+shubhamvij@users.noreply.github.com> Co-authored-by: Kyle Montemayor <kmontemayor2@snapchat.com> Co-authored-by: kmontemayor <kyle.e.montemayor@gmail.com>

svij-sc · 2026-05-14T16:24:01Z

I gave the robots a go at it and the generated https://github.com/Snapchat/GiGL/pull/597/changes - WDYT about merging it into your follow up PR?

I do feel pretty strongly about not type-ignoring torch, even if temporarily.

I pulled in the changes - thanks @kmontemayor2-sc

kmontemayor2-sc

Awesome work Shubham! Really appreciate it :)

kmontemayor2-sc · 2026-05-14T16:30:19Z

@@ -402,10 +420,12 @@ def __batch_copy_blobs(
            dst_prefix: str,
            src_blobs: list[storage.Blob],
        ):
-            dst_blob_names: list[str] = [
-                src_blob.name.replace(src_prefix, dst_prefix, 1)
-                for src_blob in src_blobs
-            ]
+            dst_blob_names: list[str] = []
+            for src_blob in src_blobs:
+                assert src_blob.name is not None, (
+                    "Blob from list_blobs must have a name"
+                )
+                dst_blob_names.append(src_blob.name.replace(src_prefix, dst_prefix, 1))


hmm is it easier to do blobs = [b for b in self.__list_file_blobs_at_gcs_path(gcs_path=gcs_path) if b.name is not None on line 213?

kmontemayor2-sc · 2026-05-14T16:37:28Z

+        if isinstance(_uri, GcsUri):
            exists = self.__gcs_utils.does_gcs_file_exist(gcs_path=_uri)
-        elif type(_uri) == LocalUri:
-            exists = does_path_exist(cast(LocalUri, _uri))
-        elif type(_uri) == HttpUri:
-            exists = HttpUtils.does_http_path_resolve(http_path=cast(HttpUri, _uri))
+        elif isinstance(_uri, LocalUri):
+            exists = does_path_exist(_uri)
+        elif isinstance(_uri, HttpUri):
+            exists = HttpUtils.does_http_path_resolve(http_path=_uri)


isn't this a real logic change?

I suppose this is better since it catches sub classes?

kmontemayor2-sc · 2026-05-14T16:42:37Z

+# on values imported from those modules.
+# https://docs.astral.sh/ty/reference/configuration/#replace-imports-with-any
+replace-imports-with-any = [
+  "apache_beam.**",


I don't think apache_beam was ignored before? I assume ty is unhappy with this? Can we test if we remove this?

svij-sc requested review from kmontemayor2-sc, mkolodner-sc, nshah-sc, xgao4-sc, yliu2-sc and zfan3-sc as code owners April 13, 2026 22:50

svij-sc and others added 3 commits April 15, 2026 00:59

Merge remote-tracking branch 'origin/main' into svij/use-ty

60dd366

revert changelog

1136c2d

svij-sc mentioned this pull request Apr 15, 2026

Implement ty fixes #586

Merged

svij-sc commented Apr 15, 2026

View reviewed changes

svij-sc added 4 commits April 15, 2026 02:14

update

ada8d42

update pyproject.toml

0e40212

update pyproject

789f1c6

format

00c05e9

kmontemayor2-sc reviewed Apr 15, 2026

View reviewed changes

svij-sc commented Apr 17, 2026

View reviewed changes

svij-sc added 3 commits April 17, 2026 23:52

address pr comments

3fd253a

merge main

db6e523

Merge remote-tracking branch 'origin/main' into svij/use-ty

6792f50

kmontemayor2-sc reviewed Apr 27, 2026

View reviewed changes

Comment thread pyproject.toml Outdated

svij-sc and others added 2 commits May 14, 2026 15:52

Merge remote-tracking branch 'origin/main' into svij/use-ty

b91fef3

Implement ty fixes (#586)

5dd0996

Co-authored-by: shubhamvij <25601958+shubhamvij@users.noreply.github.com> Co-authored-by: Kyle Montemayor <kmontemayor2@snapchat.com> Co-authored-by: kmontemayor <kyle.e.montemayor@gmail.com>

svij-sc enabled auto-merge May 14, 2026 16:39

kmontemayor2-sc approved these changes May 14, 2026

View reviewed changes

Conversation

svij-sc commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

svij-sc Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

svij-sc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kmontemayor2-sc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

svij-sc commented May 14, 2026

Uh oh!

kmontemayor2-sc left a comment

Choose a reason for hiding this comment

Uh oh!

kmontemayor2-sc May 14, 2026

Choose a reason for hiding this comment

Uh oh!

kmontemayor2-sc May 14, 2026

Choose a reason for hiding this comment

Uh oh!

kmontemayor2-sc May 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

svij-sc commented Apr 13, 2026 •

edited

Loading

svij-sc Apr 15, 2026 •

edited

Loading