-
Notifications
You must be signed in to change notification settings - Fork 140
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
disallow untyped defs #289
Conversation
6aa6949
to
a14d442
Compare
a14d442
to
010c435
Compare
7aa7b52
to
9b14209
Compare
Should there be a new wrapper function for asyncio |
3692393
to
4b76b93
Compare
4b76b93
to
ebb72c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some question, some change suggestions.
and support falsy name=""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there! Just a suggestion regarding the type of extra_attributes
, and some minor changes.
class TaskGroup(abc.TaskGroup): | ||
def __init__(self): | ||
def __init__(self) -> None: | ||
self._active = False | ||
self._nursery_manager = trio.open_nursery() | ||
self.cancel_scope = None | ||
self.cancel_scope = None # type: ignore[assignment] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the cancel_scope
member is never accessed unless the cancel scope is active, perhaps this should be declared just with its type and no default value on the class? Without setting it to None
in the initializer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's pretty much equivalent to a # type: ignore[assignment]
, but means you're assigning to attributes outside the constructor. I think the real fix is to use a TaskGroup/TaskGroupManager pattern like trio:
class TaskGroup(abc.TaskGroup):
def __init__(self, nursery: trio.Nursery):
self._active = True
self._nursery = nursery
self._cancel_scope = CancelScope(nursery.cancel_scope)
class TaskGroupManager(abc.TaskGroupManager):
def __init__(self):
self._nursery_manager = trio.open_nursery()
async def __aenter____(self):
return TaskGroup(await self._nursery_manager.__aenter__())
async def __aexit__(self, *args, **kwargs):
return await self._nursery_manager.__aexit__()
Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>
0663752
to
f3d7411
Compare
Thank you for your hard work on this! |
disallow untyped defs and fix runtime problems discovered by typing all defs:
TextStream.extra_attributes
no longer crashes with an attribute errorawait maybe_async(current_task())
no longer returns Noneanyio.maybe_async
pickle.dumps(get_current_task())
now correctly raises a TypeError (or similar on pypy) rather than pickling to Nonename
s are converted tostr
on the asyncio back-endalso type level errors:
anyio.Lock()
that were missing a constructor type now have one, see Function is missing type annotation for __init__(self) python/mypy#5943 (comment)anyio._backend._asyncio.run
takes a coroutine factory and returns the coroutine result - rather than any callableawait anyio.Event().wait()
returnsNone
at runtime, rather thanbool
. Updated type annotations to reflect thisanyio.lowlevel.RunVar().get
type annotation was updated to return either the default value or the contained type