Permit referring to some generic types in generic ways #908

oremanj · 2019-02-05T09:18:40Z

Specifically, declare SendChannel, ReceiveChannel, Listener, and RunVar to
be generic in one type parameter, and also support the open_memory_channel[T](bufsize)
syntax at runtime. Until trio is able to support typing directly, this change allows
users of external stubs to use correctly-typed channels without too many hacks.

Specifically, declare `SendChannel`, `ReceiveChannel`, `Listener`, and `RunVar` to be generic in one type parameter, and also support the `open_memory_channel[T](bufsize)` syntax at runtime. Until trio is able to support typing directly, this change allows users of external stubs to use correctly-typed channels without too many hacks.

codecov · 2019-02-05T09:36:03Z

Codecov Report

Merging #908 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #908      +/-   ##
==========================================
- Coverage    99.4%   99.36%   -0.04%     
==========================================
  Files         102      102              
  Lines       12679    12707      +28     
  Branches      926      927       +1     
==========================================
+ Hits        12603    12626      +23     
- Misses         56       60       +4     
- Partials       20       21       +1

Impacted Files	Coverage Δ
trio/tests/test_util.py	`100% <100%> (ø)`	⬆️
trio/_abc.py	`100% <100%> (ø)`	⬆️
trio/_core/_local.py	`100% <100%> (ø)`	⬆️
trio/_core/tests/test_local.py	`100% <100%> (ø)`	⬆️
trio/_channel.py	`100% <100%> (ø)`	⬆️
trio/_util.py	`93.54% <100%> (-5.27%)`	⬇️

codecov · 2019-02-05T09:36:04Z

Codecov Report

Merging #908 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #908      +/-   ##
=========================================
+ Coverage    99.4%   99.4%   +<.01%     
=========================================
  Files         102     102              
  Lines       12681   12715      +34     
  Branches      927     927              
=========================================
+ Hits        12605   12639      +34     
  Misses         56      56              
  Partials       20      20

Impacted Files	Coverage Δ
trio/tests/test_util.py	`100% <100%> (ø)`	⬆️
trio/_abc.py	`100% <100%> (ø)`	⬆️
trio/tests/test_abc.py	`100% <100%> (ø)`	⬆️
trio/_channel.py	`100% <100%> (ø)`	⬆️
trio/_util.py	`98.92% <100%> (+0.11%)`	⬆️
trio/_core/_run.py	`99.71% <0%> (-0.01%)`	⬇️
trio/_core/tests/test_run.py	`100% <0%> (ø)`	⬆️

njsmith · 2019-02-05T09:45:31Z

What's the advantage of keeping the actual stubs out of trio here?

Oh, I see the force-push. We gotta fix sphinxcontrib-trio to handle these better...

oremanj · 2019-02-05T09:52:06Z

Putting the type hints in trio currently has limited utility because in the absence of stubs mypy chokes when you try to import trio due to the _core._run magic (among other things). I wanted to include them on the generic classes because why not, but sphinx didn't do well with that. It might just fix itself if we enable the sphinx_autodoc_typehints extension, but that can be a separate PR, I think. :-)

More context here: I'm working on a trio-typing package which provides stubs for trio, async_generator, and outcome, and some other stuff like a mypy plugin to typecheck nursery.start_soon(fn, *args) correctly. I figure that until we're ready to fully tackle in-tree type hints (definitely desirable! but a much larger undertaking), the small change here can serve as a decent stopgap that lets people start experimenting.

oremanj · 2019-02-05T18:14:34Z

Looks like 3.5.0 doesn't support instantiating generic slots classes. :-( I'll remove the RunVar genericization because it's much less usability-relevant than the others: not many people create RunVars, and no one inherits from them so the workaround is just quotes rather than some elaborate if TYPE_CHECKING: thing.

njsmith

I wrote a few comments. They're mostly "thoughts that occurred to me while reading" than "critique". Overall it seems fine, and moves us incrementally in a good direction, so feel free to take them or leave them :-)

njsmith · 2019-02-06T09:33:59Z

trio/_abc.py

+
+# The type of object produced by a Listener (covariant plus must be
+# an AsyncResource)
+T_resource = TypeVar("T_resource", bound=AsyncResource, covariant=True)


I think the bound=AsyncResource will disappear when we fix #636. We could start skipping it now, or we could put it in and then take it out later, doesn't make much difference IMO.

You think we'll have Listeners that provide objects not representable as an AsyncResource? I'm a bit surprised at that, can you give an example?

I don't have an example in mind. But the reason we restrict it to AsyncResource right now is because serve_listeners automatically closes the object after the handler returns:

trio/trio/_highlevel_serve_listeners.py

Lines 25 to 29 in 0a815a0

async def _run_handler(stream, handler):

try:

await handler(stream)

finally:

await trio.aclose_forcefully(stream)

With #636, this code goes away, because the Listener itself is responsible for managing the handler lifetime. So... at that point it's really between the person developing a Listener[X] and their users, what kind of API X should support :-).

Ah, that makes sense! I think it's probably sensible to keep the AsyncResource bound for now, since it reflects the current requirements, and we can easily change it later.

trio/_abc.py

trio/_util.py

njsmith · 2019-02-06T09:41:26Z

trio/_util.py

+        return self._fn(*args, **kwargs)
+
+    def __getitem__(self, _):
+        return self


So you actually can make open_memory_channel directly typeable by mypy without any plugin, and it's not too terribly awful: python/mypy#6073 (comment)

But I think it does require inlining this class into the definition of open_memory_channel.

If that's where were going to end up, then maybe setting up a standalone @generic_function decorator, testing it, etc., is overkill, and we should instead move this code into _memory_channels.py right now?

The problem with that approach is that AFAICT, when you write Type[T], mypy can't infer T as one of its "special forms" (Tuple, Union, Callable, etc). You just silently get Any if you use such a one. I figured that was error-prone enough that we should go this route instead, at least for now.

trio/tests/test_abc.py

oremanj · 2019-02-06T20:05:10Z

CI problem is codecov.io returning an invalid script

oremanj requested a review from njsmith February 5, 2019 09:18

oremanj force-pushed the generics branch from 571f3fd to b93ed9e Compare February 5, 2019 09:20

Remove annotations from ABC methods because they look ugly in Sphinx

ac9ce99

oremanj force-pushed the generics branch from a678789 to ac9ce99 Compare February 5, 2019 09:39

oremanj added 2 commits February 5, 2019 01:58

scattershot attempt to fix 3.5.0 CI

eff920e

Add newsfragment, revert RunVar changes

25e82dd

oremanj added 2 commits February 5, 2019 10:18

add test of instantiating a SendChannel[T] subclass

77edcbb

make it slotted

2b8d2b8

oremanj mentioned this pull request Feb 5, 2019

Drop support for early python 3.5.x point releases? #880

Closed

oremanj added 2 commits February 5, 2019 10:36

yapf

22747c2

coverage

6fda343

njsmith reviewed Feb 6, 2019

View reviewed changes

oremanj added 2 commits February 6, 2019 11:45

Merge remote-tracking branch 'origin/master' into generics

870d0d3

CR comments

7c2d2d6

oremanj closed this Feb 6, 2019

oremanj reopened this Feb 6, 2019

njsmith merged commit 1dc21fa into python-trio:master Feb 8, 2019

gsalgado referenced this pull request in gsalgado/trinity Jan 7, 2020

Use ExpectedResponseChannels for pongs

c28c9e5

jakkdl mentioned this pull request Feb 1, 2023

Add generic typing support for Memory[Send/Receive]Channel #2549

Merged

	async def _run_handler(stream, handler):
	try:
	await handler(stream)
	finally:
	await trio.aclose_forcefully(stream)

Uh oh!

Permit referring to some generic types in generic ways #908

Permit referring to some generic types in generic ways #908

Uh oh!

Conversation

oremanj commented Feb 5, 2019

Uh oh!

codecov bot commented Feb 5, 2019

Codecov Report

Uh oh!

codecov bot commented Feb 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

njsmith commented Feb 5, 2019

Uh oh!

oremanj commented Feb 5, 2019

Uh oh!

oremanj commented Feb 5, 2019

Uh oh!

njsmith left a comment

Choose a reason for hiding this comment

Uh oh!

njsmith Feb 6, 2019

Choose a reason for hiding this comment

Uh oh!

oremanj Feb 6, 2019

Choose a reason for hiding this comment

Uh oh!

njsmith Feb 6, 2019

Choose a reason for hiding this comment

Uh oh!

oremanj Feb 6, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

njsmith Feb 6, 2019

Choose a reason for hiding this comment

Uh oh!

oremanj Feb 6, 2019

Choose a reason for hiding this comment

Uh oh!

njsmith Feb 6, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

oremanj commented Feb 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Feb 5, 2019 •

edited

Loading