Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Improve type hints for attrs classes #16276

Merged
merged 6 commits into from
Sep 8, 2023
Merged

Improve type hints for attrs classes #16276

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0bd78b1
_LinearizerEntry.deferred: key and value types
Sep 7, 2023
a35fa4f
Generic attrs classes I could spot
Sep 7, 2023
21571d1
Pattern->Pattern[str]
Sep 7, 2023
0558c1e
Annotate DoneAwaitable
Sep 7, 2023
b12a786
Changelog
Sep 7, 2023
a611a80
Fix ExpiringCache Sized complaints
Sep 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 9 additions & 13 deletions synapse/util/async_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
Collection,
Coroutine,
Dict,
Generator,
Generic,
Hashable,
Iterable,
Expand Down Expand Up @@ -718,30 +719,25 @@ def failure_cb(val: Failure) -> None:
return new_d


# This class can't be generic because it uses slots with attrs.
# See: https://github.com/python-attrs/attrs/issues/313
@attr.s(slots=True, frozen=True, auto_attribs=True)
class DoneAwaitable: # should be: Generic[R]
class DoneAwaitable(Awaitable[R]):
"""Simple awaitable that returns the provided value."""

value: Any # should be: R
value: R

def __await__(self) -> Any:
return self

def __iter__(self) -> "DoneAwaitable":
return self

def __next__(self) -> None:
raise StopIteration(self.value)
def __await__(self) -> Generator[Any, None, R]:
yield None
Copy link
Member

Choose a reason for hiding this comment

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

Not sure it matters, but is the yield less efficient?

I'm not 100% sure I understand removing __iter__ and __next__.

Copy link
Contributor Author

@DMRobertson DMRobertson Sep 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Err, being honest it's because I couldn't work out how to annotate the __iter__ and __next__ in a way that mypy was happy with. Can dig up the errors if you'd like.

I think this is equivalent though:

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I think it wasn't redundant because we were returning ourself, now __await__ itself is an iterator?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I think it wasn't redundant because we were returning ourself, now __await__ itself is an iterator?

This explains why __next__ was needed before my change. But I assert that __iter__ is redundant these days:

$ python3.8 -m asyncio
asyncio REPL 3.8.17 (default, Jun  8 2023, 00:00:00) 
[GCC 13.1.1 20230511 (Red Hat 13.1.1-2)] on linux
Use "await" directly instead of "asyncio.run()".
Type "help", "copyright", "credits" or "license" for more information.
>>> import asyncio
>>> class DummyAwaitable:
...     def __init__(self, value):
...         self.value = value
...     
...     def __await__(self):
...         return self
...     
...     def __next__(self):
...         raise StopIteration(self.value)
... 
>>> 
>>> await DummyAwaitable(1234)
1234

AFAICS you used to need __iter__ so that you could yield from, back in the days when async wasn't a keyword (a = yield from b instead of a = await b.) If I try to use my class in an old-style coroutine:

>>> import asyncio
>>> @asyncio.coroutine
... def f():
...     val = yield from DummyAwaitable(5678)
...     return val
... 
<console>:2: DeprecationWarning: "@coroutine" decorator is deprecated since Python 3.8, use "async def" instead
>>> await f()
Traceback (most recent call last):
  File "/usr/lib64/python3.8/concurrent/futures/_base.py", line 444, in result
    return self.__get_result()
  File "/usr/lib64/python3.8/concurrent/futures/_base.py", line 389, in __get_result
    raise self._exception
  File "<console>", line 1, in <module>
  File "<console>", line 3, in f
TypeError: 'DummyAwaitable' object is not iterable

But if we define __iter__:

>>> class OldDummy(DummyAwaitable):
...     def __iter__(self):
...         return self
... 
>>> @asyncio.coroutine
... def g():
...     val = yield from OldDummy(5678)
...     return val
... 
<console>:2: DeprecationWarning: "@coroutine" decorator is deprecated since Python 3.8, use "async def" instead
>>>     
>>> await g()
5678

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Tbh we could probably use asyncio.Future instead of writing our own)

Copy link
Member

Choose a reason for hiding this comment

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

(Tbh we could probably use asyncio.Future instead of writing our own)

Alas, Future docs say:

Deprecated since version 3.10: Deprecation warning is emitted if loop is not specified and there is no running event loop.

return self.value
Comment on lines +723 to +730
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't resist trying to simplify this. Needs careful checking though, I find all this confusing.

C.f. a0aef0b



def maybe_awaitable(value: Union[Awaitable[R], R]) -> Awaitable[R]:
"""Convert a value to an awaitable if not already an awaitable."""
if inspect.isawaitable(value):
assert isinstance(value, Awaitable)
return value

# For some reason mypy doesn't deduce that value is not Awaitable here, even though
# inspect.isawaitable returns a TypeGuard.
assert not isinstance(value, Awaitable)
return DoneAwaitable(value)


Expand Down