https://codereview.appspot.com/67400043/diff/40001/asyncio/base_events.py
File asyncio/base_events.py (right):

https://codereview.appspot.com/67400043/diff/40001/asyncio/base_events.py#newcode240
asyncio/base_events.py:240: def call_later(self, delay, callback, *args,
_check_loop=True):
I don't understand why you added a parameter here. I don't see why you
would call call_later(..., _check_loop=False)

https://codereview.appspot.com/67400043/diff/40001/asyncio/base_events.py#newcode257
asyncio/base_events.py:257: self._assert_is_current_event_loop()
Why not relying on call_at() check? It would avoid the need of adding a
private parameter.

https://codereview.appspot.com/67400043/diff/40001/asyncio/base_events.py#newcode261
asyncio/base_events.py:261: def call_at(self, when, callback, *args,
_check_loop=True):
IMO you should remove the parameter.

https://codereview.appspot.com/67400043/diff/40001/asyncio/base_events.py#newcode265
asyncio/base_events.py:265: if _check_loop:
You can move the check on the debug flag here, for better performances:

if self._debug:
   self._check_event_loop()

https://codereview.appspot.com/67400043/diff/40001/asyncio/base_events.py#newcode271
asyncio/base_events.py:271: def call_soon(self, callback, *args,
_check_loop=True):
I don't like this private parameter, I would prefer a new submethod:

def _call_soon(self, callback, args, check_event_loop):
  if self._debug and check_event_loop:
    self._check_event_loop()
  ...

And call it from call_soon_threadsafe() and call_soon().

https://codereview.appspot.com/67400043/diff/40001/asyncio/base_events.py#newcode289
asyncio/base_events.py:289: def _assert_is_current_event_loop(self):
I don't understand the term "assert" in the function name you proposed,
it's not an assertion (but you used AssertionError in a previous version
of your patch).

I propose to rename the method to "_check_event_loop".

You can move it up to define it before you use it, so before
call_later().

https://codereview.appspot.com/67400043/diff/40001/asyncio/base_events.py#newcode290
asyncio/base_events.py:290: if self.get_debug() and
events.get_event_loop() != self:
You may write "events.get_event_loop() is not self".

(Drop self.get_debug() test, move it to the caller as self._debug.)

Note: Calling events.get_event_loop() in the main thread creates an
event loop if the current event loop is running in a dedicated thread
and the main thread didn't have an event loop. Since the check is only
done in debug mode, it's fine.

https://codereview.appspot.com/67400043/diff/40001/asyncio/base_events.py#newcode292
asyncio/base_events.py:292: "non-threadsafe operation invoked on event
loop other than " +
"on *an* event loop"?

https://codereview.appspot.com/67400043/diff/40001/asyncio/test_utils.py
File asyncio/test_utils.py (right):

https://codereview.appspot.com/67400043/diff/40001/asyncio/test_utils.py#newcode353
asyncio/test_utils.py:353: def call_at(self, when, callback, *args,
**kwargs):
This is exactly why I'm against using private keyword parameter, it
makes the API uglier.

https://codereview.appspot.com/67400043/diff/40001/tests/test_base_events.py
File tests/test_base_events.py (right):

https://codereview.appspot.com/67400043/diff/40001/tests/test_base_events.py#newcode159
tests/test_base_events.py:159: (wrong_thread and debug) else
EmptyContextManager()
Other may disagree, but why not only testing the case which must raise
RuntimeError? It would avoid EmptyContextManager.

https://codereview.appspot.com/67400043/diff/40001/tests/test_base_events.py#newcode941
tests/test_base_events.py:941:
(Space) change unrelated to the issue, it should be reverted.

https://codereview.appspot.com/67400043/

Reply via email to