Misc type hint fixes by stephenfin · Pull Request #278 · sqlalchemy/dogpile.cache

stephenfin · 2025-10-01T12:03:47Z

A collection of typing-related changes built while typing oslo.cache.

zzzeek

oh. wow OK, I had forgotten that we hadn't turned on strict typing for dogpile.

can you do me a favor and let's turn on a little bit of strict - if i do this on the current main, it produces only 19 errors which are likely mostly handled here:

diff --git a/pyproject.toml b/pyproject.toml
index f8a77a2..7e0e293 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -58,6 +58,17 @@ valkey = [
 [project.entry-points."mako.cache"]
 "dogpile.cache" = "dogpile.cache.plugins.mako_cache:MakoPlugin"
 
+[tool.mypy]
+show_error_codes = true
+incremental = true
+
+strict = true
+
+disallow_untyped_defs = false
+disallow_untyped_calls = false
+ignore_missing_imports = true
+
+
 [tool.setuptools]
 zip-safe = false
 include-package-data = true

This requires we remove the old mypy.ini file.

The collections.abc changes here are surprising to me and I dont see mypy errors locally for those, can you provide background on those? I dont see what's wrong with the typing version of those collections.

stephenfin · 2025-10-01T12:43:02Z

oh. wow OK, I had forgotten that we hadn't turned on strict typing for dogpile.

can you do me a favor and let's turn on a little bit of strict - if i do this on the current main, it produces only 19 errors which are likely mostly handled here:
diff --git a/pyproject.toml b/pyproject.toml
index f8a77a2..7e0e293 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -58,6 +58,17 @@ valkey = [
 [project.entry-points."mako.cache"]
 "dogpile.cache" = "dogpile.cache.plugins.mako_cache:MakoPlugin"
 
+[tool.mypy]
+show_error_codes = true
+incremental = true
+
+strict = true
+
+disallow_untyped_defs = false
+disallow_untyped_calls = false
+ignore_missing_imports = true
+
+
 [tool.setuptools]
 zip-safe = false
 include-package-data = true
This requires we remove the old mypy.ini file.

Will do 👌

The collections.abc changes here are surprising to me and I dont see mypy errors locally for those, can you provide background on those? I dont see what's wrong with the typing version of those collections.

They're one of those things (like List and Tuple) that are deprecated since the stdlib has caught up and made them subscriptable. I've called it out in the commit message.

zzzeek · 2025-10-01T13:05:55Z

The collections.abc changes here are surprising to me and I dont see mypy errors locally for those, can you provide background on those? I dont see what's wrong with the typing version of those collections.

They're one of those things (like List and Tuple) that are deprecated since the stdlib has caught up and made them subscriptable. I've called it out in the commit message.

FTR that link is https://docs.python.org/3/library/typing.html#aliases-to-container-abcs-in-collections-abc

wow that's going to mess with me

sqla-tester

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision c798805 of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester · 2025-10-01T14:01:01Z

New Gerrit review created for change c798805: https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/6182

stephenfin · 2025-10-01T14:42:07Z

CI failures were because I used the new union syntax instead of typing.Union. Will fix.

All of these things are unnecessary in Python 3.9 [1]. [1] https://docs.python.org/3/library/typing.html#aliases-to-container-abcs-in-collections-abc Signed-off-by: Stephen Finucane <stephen@that.guru>

sqla-tester

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 163b4e9 of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester · 2025-10-01T15:53:58Z

Patchset 163b4e9 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/6182

Signed-off-by: Stephen Finucane <stephen@that.guru>

This keeps both mypy and flake8 happy. Win win. Signed-off-by: Stephen Finucane <stephen@that.guru>

Signed-off-by: Stephen Finucane <stephen@that.guru>

We move the configuration to pyproject.toml and enable strict mode (though we disable some settings since we're not fully typed yet). While here we also remove a redundant setting for black: black (and ruff) will automatically infer the target version from '[project.requires-python]' Signed-off-by: Stephen Finucane <stephen@that.guru>

sqla-tester

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision aa04d4d of this pull request into gerrit so we can run tests and reviews and stuff

sqla-tester · 2025-10-01T18:23:36Z

Patchset aa04d4d added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/6182

zzzeek · 2025-10-01T19:49:56Z

i've got some other things moving in dogpile.cache where you'd need a rebase on this, but if this is ready I can just finalize in gerrit and merge

stephenfin · 2025-10-01T19:52:57Z

I think this is ready. There's more that'll likely pop up since we're not fully typed but I won't be back on this for another week or two.

zzzeek · 2025-10-01T20:02:51Z

ok great.

sqla-tester · 2025-10-01T20:22:14Z

Michael Bayer (zzzeek) wrote:

recheck

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/6182

stephenfin · 2025-10-02T09:57:50Z

@zzzeek I'm not sure if that error is related to this change? It doesn't appear to be, especially given I'm not making any functional changes. Another recheck needed?

A Sequence requires the ability to index (__getitem__) and retrieve the length (__len__), amongst other options. An Iterable simply required the ability to iterate (__iter__). This allows us to pass, amongst other things, dict_keys, to the 'get_multi', 'get_serialized_multi', and 'delete_multi' methods, which seems like a rational thing to want and be able to do. Signed-off-by: Stephen Finucane <stephen@that.guru>

stephenfin · 2025-10-02T11:05:49Z

I think this is ready. There's more that'll likely pop up since we're not fully typed but I won't be back on this for another week or two.

I tell a lie: another thing popped up yesterday. I've just pushed it (I want to be able to pass the value of dict.keys() to get_multi, since we do that in a few places in oslo.cache).

This is helpful if the expiration_time callable relies on some global configuration, for example. Signed-off-by: Stephen Finucane <stephen@that.guru>

zzzeek · 2025-10-02T13:59:21Z

do you have access to push to the gerrit? since you're gerrit enabled. I already put a little changelog into it. the failures are because I'm messing with the CI and added a tox target, so I just rebased the gerrit to hopefully get those.

zzzeek · 2025-10-02T20:33:51Z

 ]

-ExpirationTimeCallable = Callable[[], float]
+ExpirationTimeCallable = Callable[[], Optional[float]]


yup looks like the way it's used already expects None

zzzeek · 2025-10-02T20:37:21Z

sure. these are all collections of keys that we only iterate. wonder why I put Sequence there.

zzzeek · 2025-10-02T20:38:05Z

zzzeek · 2025-10-02T20:39:36Z

        pickle.dumps
-    )
-    deserializer: Union[None, Deserializer] = staticmethod(  # type: ignore
+    )  # type: ignore


let me turn on warn_unused_ignores for these

wow, no errors, OK.

sqla-tester · 2025-10-02T20:58:32Z

Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/6182 has been merged. Congratulations! :)

stephenfin force-pushed the typing branch from 992329f to 4474ccb Compare October 1, 2025 12:09

zzzeek requested changes Oct 1, 2025

View reviewed changes

stephenfin force-pushed the typing branch from 4474ccb to c798805 Compare October 1, 2025 12:49

zzzeek requested a review from sqla-tester October 1, 2025 14:00

sqla-tester reviewed Oct 1, 2025

View reviewed changes

Replace deprecated typing imports

2247a42

All of these things are unnecessary in Python 3.9 [1]. [1] https://docs.python.org/3/library/typing.html#aliases-to-container-abcs-in-collections-abc Signed-off-by: Stephen Finucane <stephen@that.guru>

stephenfin force-pushed the typing branch from c798805 to 163b4e9 Compare October 1, 2025 15:02

stephenfin mentioned this pull request Oct 1, 2025

Infer python_version from project.requires-python when using pyproject.toml python/mypy#19349

Open

zzzeek requested a review from sqla-tester October 1, 2025 15:53

sqla-tester reviewed Oct 1, 2025

View reviewed changes

stephenfin added 5 commits October 1, 2025 17:45

Add typing to util module

5ae3160

Signed-off-by: Stephen Finucane <stephen@that.guru>

Add typing to region module

d09bbaa

Signed-off-by: Stephen Finucane <stephen@that.guru>

Explicitly export symbols

304a772

This keeps both mypy and flake8 happy. Win win. Signed-off-by: Stephen Finucane <stephen@that.guru>

Remove unnecessary type ignores

17c1ee4

Signed-off-by: Stephen Finucane <stephen@that.guru>

stephenfin force-pushed the typing branch from 163b4e9 to aa04d4d Compare October 1, 2025 16:46

zzzeek requested a review from sqla-tester October 1, 2025 18:23

sqla-tester reviewed Oct 1, 2025

View reviewed changes

Allow expiration_time callable to return None

69771c6

This is helpful if the expiration_time callable relies on some global configuration, for example. Signed-off-by: Stephen Finucane <stephen@that.guru>

zzzeek reviewed Oct 2, 2025

View reviewed changes

Comment thread mypy.ini

Copy link
Copy Markdown

Member

zzzeek Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

zzzeek reviewed Oct 2, 2025

View reviewed changes

sqlalchemy-bot closed this in 44ef974 Oct 2, 2025

Conversation

stephenfin commented Oct 1, 2025

Uh oh!

zzzeek left a comment

Choose a reason for hiding this comment

Uh oh!

stephenfin commented Oct 1, 2025

Uh oh!

zzzeek commented Oct 1, 2025

Uh oh!

sqla-tester left a comment

Choose a reason for hiding this comment

Uh oh!

sqla-tester commented Oct 1, 2025

Uh oh!

stephenfin commented Oct 1, 2025

Uh oh!

sqla-tester left a comment

Choose a reason for hiding this comment

Uh oh!

sqla-tester commented Oct 1, 2025

Uh oh!

sqla-tester left a comment

Choose a reason for hiding this comment

Uh oh!

sqla-tester commented Oct 1, 2025

Uh oh!

zzzeek commented Oct 1, 2025

Uh oh!

stephenfin commented Oct 1, 2025 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zzzeek commented Oct 1, 2025

Uh oh!

sqla-tester commented Oct 1, 2025

Uh oh!

stephenfin commented Oct 2, 2025

Uh oh!

stephenfin commented Oct 2, 2025

Uh oh!

zzzeek commented Oct 2, 2025

Uh oh!

zzzeek Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

zzzeek Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

zzzeek Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

zzzeek Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

zzzeek Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

sqla-tester commented Oct 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stephenfin commented Oct 1, 2025 via email •

edited

Loading