Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: return "Self" from "BaseRetrying.copy" #518

Merged
merged 1 commit into from
Mar 5, 2025

Conversation

ThirVondukr
Copy link
Contributor

Currently BaseRetrying.copy returns BaseRetrying, making reusing a "template"/base retrying object harder.
Example usecase:

from tenacity import AsyncRetrying


class SomeClient:
    def __init__(self, retry: AsyncRetrying) -> None:
        self._retry = retry

    async def method(self) -> None:
        async for attempt in self._retry.copy():
            with attempt:
                pass  # Do something
$ mypy main.py 
main.py:9: error: "BaseRetrying" has no attribute "__aiter__" (not async iterable)  [attr-defined]
Found 1 error in 1 file (checked 1 source file)

Copy link
Contributor

mergify bot commented Mar 3, 2025

⚠️ No release notes detected. Please make sure to use reno to add a changelog entry.

@ThirVondukr
Copy link
Contributor Author

@jd On an unrelated note - would a PR that adds a package manager (uv seems to be a good choice nowadays) to setup a dev environment be welcome?
Also would be nice to set up https://docs.pypi.org/trusted-publishers/

@jd
Copy link
Owner

jd commented Mar 4, 2025

@jd On an unrelated note - would a PR that adds a package manager (uv seems to be a good choice nowadays) to setup a dev environment be welcome?

Yes, if that can replace tox I'll be happy.

Also would be nice to set up https://docs.pypi.org/trusted-publishers/

I'll work on that at some point, good thing.

@ThirVondukr
Copy link
Contributor Author

@jd On an unrelated note - would a PR that adds a package manager (uv seems to be a good choice nowadays) to setup a dev environment be welcome?

Yes, if that can replace tox I'll be happy.

If you need to run multiple python environments/versions/dependency versions tox is probably the right choice, but that could instead be done in CI and mypy/ruff seem to work correctly when you specify target version, though you still can run them in CI with all python versions.

@ThirVondukr
Copy link
Contributor Author

I've checked with mypy, pyright and PyCharm:
PyCharm seems to derive copied type correctly:
image
Mypy works fine:

from tenacity import AsyncRetrying, Retrying

retrying = AsyncRetrying()
reveal_type(retrying.copy()) # note: Revealed type is "tenacity.asyncio.AsyncRetrying"

retrying_sync = Retrying()
reveal_type(retrying_sync.copy()) # note: Revealed type is "tenacity.Retrying"

Pyright too:

src\main.py:4:13 - information: Type of "retrying.copy()" is "AsyncRetrying"
src\main.py:7:13 - information: Type of "retrying_sync.copy()" is "Retrying"

Typing extensions has section in documentation about using types in runtime: https://typing-extensions.readthedocs.io/en/latest/#runtime-use-of-types
It would only be a problem here if BaseRetrying.copy would be inspected for some reason:

import typing

from tenacity import AsyncRetrying, Retrying

typing.get_type_hints(AsyncRetrying.copy)

but is seems to fail earlier due to StopBaseT not being defined since it's imported in a type-checking block too, so adding Self without typing_extensions doesn't seem to be a problem here.

@jd jd added the no-changelog No changelog needed label Mar 5, 2025
@mergify mergify bot merged commit bfbf173 into jd:main Mar 5, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog No changelog needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants