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

Added generator send method #823

Merged
merged 15 commits into from Jun 3, 2018

Conversation

Projects
None yet
2 participants
@BPYap
Contributor

BPYap commented May 25, 2018

This PR enables yield keyword to be used in expression and introduces send method to generator in voc.

Some example of things you can do with yield expression and send method:

  1. Echoes what is sent
def gen():
    # echoes what is sent in
    while True:
        x = print((yield)) 

g = gen()
next(g)
g.send("Hi there!")
g.send("Sup")

Output:

Hi there!
Sup
  1. Yield square of value sent
def gen():
    # yield square of value sent
    while True:
        x = yield("ready") 
        yield x ** 2

g = gen()
next(g)
print(g.send(2))
next(g)
print(g.send(-1))

Output:

4
1
  1. Comparing value sent into it
def gen():
    while True:
        x = 1 < (yield) < 5
        print(x)

g = gen()
next(g)
g.send(1)
g.send(3)

Output:

False
True

Note:

  • Once this PR and #821 are confirmed stable, I will implement the same features to yield from keyword

BPYap added some commits May 24, 2018

Makes yield keyword requires no operand
`yield` will behave like `yield None`
@freakboy3742

This is a good start. The test cases you've added are great.

I've left a couple of high level comments - mostly a need for clarification on exactly how you're handling some edge cases. I imagine I'll probably have some more comments once those high level questions have been addressed.

def unused():
yield from range(5)
print('Hello, world!')
print('Hello, world!')

This comment has been minimized.

@freakboy3742

freakboy3742 May 25, 2018

Member

The (lack of) indentation on this line was intentional. The test needs some output to verify that something ran. The dummy "Hello world" is proof that the code ran. If it's in the unused() method, but unused() isn't invoked, then the print will never happen.

This entire test test_yield_from_not_used case will also become redundant as soon as yield from exists; the purpose of this test is to validate that just having yield from in your code won't cause a problem, as long as the yield from isn't actually on a live code path.

This comment has been minimized.

@freakboy3742

freakboy3742 Jun 1, 2018

Member

To clarify; this change either needs to be reverted, with the patch from #821 deleting the test entirely.

def gen():
while True:
x = yield 1
yield x ** 2

This comment has been minimized.

@freakboy3742

freakboy3742 May 25, 2018

Member

Is there any particular reason why a "power" operator is being used here? Is it just a mathematical operator to modify the value a bit? In terms of test naming, it seems like there is some sort of deeper significance to the choice of power specifically.

This comment has been minimized.

@BPYap

BPYap Jun 1, 2018

Contributor

I noticed pow is handled differently in visit_BinOp so I added this test case to see its behavior, it executes as intended and I have merged this test case under test_generator_yield_expr_binary_op

@@ -325,8 +325,79 @@ def visit_Delete(self, node):
@node_visitor
def visit_Assign(self, node):
# Evaluate the value
self.visit(node.value)
def push_message(node):

This comment has been minimized.

@freakboy3742

freakboy3742 May 25, 2018

Member

This shouldn't be an closure method - it's not dependent on any of the context provided by visit_Assign. You can move it into a utility method at the level of the visitor (like e.g., full_classspec).

This comment has been minimized.

@BPYap

BPYap May 25, 2018

Contributor

Yes. Completely agree with that, I realized expression is more than just assignment and augmented assignment. I'm currently working on turning the block of code into an utility function which can be called in any node that evaluate an expression.

# walk subtree with node.value as root to find any yield node
nested_yield_node_found = False
for descendant_node in ast.walk(node.value):

This comment has been minimized.

@freakboy3742

freakboy3742 May 25, 2018

Member

Is there a reason that this is handled as a separate walk, rather than just visiting node.value within the existing AST visitor that we're evaluating?

JavaOpcodes.INVOKEINTERFACE('org/python/Object', 'byValue', args=[], returns='Lorg/python/Object;')
)
elif hasattr(node, "lineno"): # a yield expression
# push NoneType object to stack

This comment has been minimized.

@freakboy3742

freakboy3742 May 25, 2018

Member

It's not clear to me why a None is needed on the stack here - is something consuming it later?

(To that end, comments like "push None onto stack" aren't that helpful - it's fairly easy to see that the next line is "add none to stack"; the comment needs to say why that is the required course of action)

This comment has been minimized.

@BPYap

BPYap Jun 1, 2018

Contributor

This is to handle the statement yield which is equivalent to yield None . I have added a comment to it in latest PR

BPYap added some commits May 25, 2018

Refactored code
Almost complete support for yield expression
@freakboy3742

Ok - this now has a merge conflict with #821; if you can resolve that conflict, this is ready to merge.

@BPYap

This comment has been minimized.

Contributor

BPYap commented Jun 3, 2018

@freakboy3742 Conflicts resolved as per request :-)

@freakboy3742 freakboy3742 merged commit 8be82ab into pybee:master Jun 3, 2018

5 checks passed

beekeeper:0/beefore:javacheckstyle Java lint checks passed.
Details
beekeeper:0/beefore:pycodestyle Python lint checks passed.
Details
beekeeper:1/smoke-test Smoke build (Python 3.4) passed.
Details
beekeeper:2/full-test:py3.5 Python 3.5 tests passed.
Details
beekeeper:2/full-test:py3.6 Python 3.6 tests passed.
Details

@BPYap BPYap deleted the BPYap:generator_send_method branch Jun 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment