Make methods of abstract base class async #2381
Conversation
DimitriPapadopoulos
force-pushed
the
async
branch
3 times, most recently
from
e86ee25
to
694af4b
Compare
|
I think a better type for these functions would be: async def list(self) -> AsyncIterator[str]:Most implementations probably won't need these functions to be async, but it's a more general type for some implementation that may need it. |
DimitriPapadopoulos
force-pushed
the
async
branch
2 times, most recently
from
46dc84b
to
52e66c9
Compare
The roadmap defines the Store API as follows and should be updated: async def list(self) -> List[str]:
... # required for listable stores
async def list_prefix(self, prefix: str) -> List[str]:
... # required for listable stores
async def list_dir(self, prefix: str) -> List[str]:
... # required for listable stores
As for the return value, not sure why it changed from |
Sorry, I just missed the argument in the example, definitely we shouldn't drop it. My point was about returning |
|
Meanwhile, I tried to revert the return type from As for the |
It's definitely not a big deal, |
|
OK, let me try then. It's just that's I'll have to modify all child classes. |
The rationale is that the methods of child classes should match the signatures of the methods of the abstract base class, including async.
`None` was added in 0e035fb, let's try to remove it.
DimitriPapadopoulos
force-pushed
the
async
branch
3 times, most recently
from
fd79078
to
844ccad
Compare
Refactor Store list methods.
DimitriPapadopoulos
force-pushed
the
async
branch
from
844ccad
to
8d3ead3
Compare
| """ | ||
| ... | ||
| return iter([]) |
There was a problem hiding this comment.
weird, you shouldn't need the implementation if you annotate with abstractmethod
| if p.is_file(): | ||
| yield str(p).replace(to_strip, "") | ||
| allfiles = [str(p).replace(to_strip, "") for p in self.root.rglob("*") if p.is_file()] | ||
| return (onefile for onefile in allfiles) |
There was a problem hiding this comment.
we don't want to materialize the full allfiles list in memory. Can you use async for instead? Here is an example of how you could do this:
from collections.abc import AsyncIterator
from abc import ABC, abstractmethod
class Foo(ABC):
@abstractmethod
async def foo(self) -> AsyncIterator[str]:
...
class Bar(Foo):
async def helper(self) -> AsyncIterator[str]:
raise ValueError("")
async def foo(self) -> AsyncIterator[str]:
return (x async for x in await self.helper())This code should work and mypy successfully, and doesn't materialize the results of calling helper.
Fixes #2377.
See microsoft/pyright#4741.
TODO: