bpo-32604: PEP 554 for use in test suite #19985
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I intentionally left out support for |
|
ohh waiting on send and recv is non trivial I guess like in https://github.com/python/cpython/pull/19829/files . |
ericsnowcurrently
requested changes
May 8, 2020
There was a problem hiding this comment.
Thanks for working on this, @nanjekyejoannah! I left a number of comments for you. Hopefully it helps. I'd be glad to chat about any of this if you have questions.
FYI, I haven't looked closely at the tests yet.
|
When you're done making the requested changes, leave the comment: |
vstinner
changed the title
nanjekyejoannah
commented
May 15, 2020
There was a problem hiding this comment.
@ericsnowcurrently, I made the requested changes and I think I left some notes on making id fields read-only for SendChannel and RecvChannel. And about the high-level implementation of Exceptions. well, we can merge a minimal version and I can follow up with more PRs after feature freeze.
ericsnowcurrently
requested changes
May 16, 2020
There was a problem hiding this comment.
Thanks for taking the time to make those changes, @nanjekyejoannah. I've left a few more comments, but it mostly looks good. Like you said, we can add onto this after it is merged.
One other thing I recommend doing in this PR is leaving a comment at each place where the implementation is blocked on low-level implementation (e.g. channel_send_wait()).
|
@ericsnowcurrently It looks like we can not directly use Does this look good to you? |
|
@nanjekyejoannah, sorry I wasn't more clear. I meant _NOT_SET = object()
class RecvChannel:
...
def recv_nowait(self, default=_NOT_SET):
"""
Like recv(), but return the default
instead of waiting.
"""
if default is _NOT_SET:
return _interpreters.channel_recv(self._id)
else:
return _interpreters.channel_recv(self._id, default) |
ericsnowcurrently
requested changes
May 18, 2020
There was a problem hiding this comment.
Thanks for making more changes. Sorry for adding extra work. It looks like there's only a handful of things left to do and we can merge this:
- sort out
_NOT_SETfor recv_nowait()` - add comments for the pieces that are not implemented yet in the low-level module
- remove the changes (2 added exceptions) in
Modules/_xxsubinterpretersmodule.c
Other than that, I've left a bunch of comments for formatting (PEP 8) changes (though they don't relate to correctness).
I considered recommending that we move the testing helpers to test.support, but we can worry about that later. 🙂
I also considered recommending that we limit the tests on the new interpreters module to just verifying integration with the low-level module. However, there is merit to having functional tests here. So what you have looks good.
…T in recv_nowait()
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @ericsnowcurrently: please review the changes made to this pull request. |
ericsnowcurrently
approved these changes
May 19, 2020
There was a problem hiding this comment.
Looks good. There are a few things to adjust, particularly once we finish the low-level module. However, what you have here gives us nearly everything else. Thanks for working on this!
Feel free to merge this after Łukasz lets everyone know that beta1 is done (and the 3.9 branch created). I expect that will be tomorrow.
Thanks for the review @ericsnowcurrently . I will merge this tommorrow. |
arturoescaip
pushed a commit
to arturoescaip/cpython
that referenced
this pull request
May 24, 2020
* PEP 554 for use in test suite * 📜🤖 Added by blurb_it. * Fix space * Add doc to doc tree * Move to modules doc tree * Fix suspicious doc errors * Fix test__all * Docs docs docs * Support isolated and fix wait * Fix white space * Remove undefined from __all__ * Fix recv and add exceptions * Remove unused exceptions, fix pep 8 formatting errors and fix _NOT_SET in recv_nowait() Co-authored-by: nanjekyejoannah <joannah.nanjekye@ibm.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
|
Seems that https://buildbot.python.org/all/#/builders/394/builds/94 ...... |
pablogsal
added a commit
to pablogsal/cpython
that referenced
this pull request
May 27, 2020
This reverts commit 9d17cbf.
|
|
nanjekyejoannah
added a commit
that referenced
this pull request
Jun 10, 2020
…)" (GH-20611) * PEP 554 for use in test suite * 📜🤖 Added by blurb_it. * Fix space * Add doc to doc tree * Move to modules doc tree * Fix suspicious doc errors * Fix test__all * Docs docs docs * Support isolated and fix wait * Fix white space * Remove undefined from __all__ * Fix recv and add exceptions * Remove unused exceptions, fix pep 8 formatting errors and fix _NOT_SET in recv_nowait() * Update Lib/test/support/interpreters.py Co-authored-by: Pablo Galindo <Pablogsal@gmail.com> * Remove documentation (module is for internal use) Co-authored-by: nanjekyejoannah <joannah.nanjekye@ibm.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
arun-mani-j
pushed a commit
to arun-mani-j/cpython
that referenced
this pull request
Jul 21, 2020
…GH-19985)" (pythonGH-20611) * PEP 554 for use in test suite * 📜🤖 Added by blurb_it. * Fix space * Add doc to doc tree * Move to modules doc tree * Fix suspicious doc errors * Fix test__all * Docs docs docs * Support isolated and fix wait * Fix white space * Remove undefined from __all__ * Fix recv and add exceptions * Remove unused exceptions, fix pep 8 formatting errors and fix _NOT_SET in recv_nowait() * Update Lib/test/support/interpreters.py Co-authored-by: Pablo Galindo <Pablogsal@gmail.com> * Remove documentation (module is for internal use) Co-authored-by: nanjekyejoannah <joannah.nanjekye@ibm.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #22 is about making use of subinterpreters (specifically the PEP 554 API) in the CPython test suite.
Until PEP 554 is accepted (or if it isn't) we should add a PEP 554 implementation as Lib/test/support/_interpreters.py (e.g. test.support._interpreters).
https://bugs.python.org/issue39881