Skip to content
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

allow futures to be awaited and yielded from #78

Merged
merged 1 commit into from Mar 2, 2019

Conversation

Projects
None yet
2 participants
@lime-green
Copy link
Contributor

commented Feb 19, 2019

Currently Pykka futures are not compatible with the await and yield from syntax. This PR simply adds that capability by implementing __await__ and __iter__ on the base Future class

@lime-green

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2019

Darn, 2.7 doesn't support returning inside a generator, and 3.7 doesn't support StopIteration inside one. I'll have to think about how to solve that

Show resolved Hide resolved tests/conftest.py Outdated
Show resolved Hide resolved tests/test_future_py3.py Outdated
Show resolved Hide resolved tests/test_future_py3.py Outdated
@jodal

jodal approved these changes Feb 19, 2019

Copy link
Owner

left a comment

👍 Thanks! I love the feature, and great work on the integration into the new pytest setup!

@jodal jodal added this to the v2.0 milestone Feb 19, 2019

@lime-green

This comment has been minimized.

Copy link
Contributor Author

commented Feb 20, 2019

@jodal Ok I resolved all the issues 😅 Thanks for your quick review :)

@lime-green

This comment has been minimized.

Copy link
Contributor Author

commented Feb 26, 2019

@jodal anything I should do to get this merged?

@jodal

This comment has been minimized.

Copy link
Owner

commented Feb 26, 2019

No, I don't think so. I've been away for a few days, so I haven't gotten around to this yet.

@jodal

jodal approved these changes Mar 2, 2019

Copy link
Owner

left a comment

Great if you can look at some of these minor comments and squash your commits. If not, I'll give it a go myself at the end of the weekend.

@@ -25,3 +25,6 @@ wheel

# Securely upload packages to PyPI
twine

# Python 3.4 doesn't have asyncio in the stdlib
asyncio; python_version == '3.4'

This comment has been minimized.

Copy link
@jodal

jodal Mar 2, 2019

Owner

Actually, asyncio is in the 3.4 stdlib. Python 3.3, now end-of-lifed, is the only Python 3.x version supported by asyncio where it wasn't part of the stdlib. Quoting https://pypi.org/project/asyncio/:

This version is only relevant for Python 3.3, which does not include asyncio in its stdlib.

This comment has been minimized.

Copy link
@lime-green

lime-green Mar 2, 2019

Author Contributor

Thanks! My mistake

Show resolved Hide resolved MANIFEST.in Outdated
Show resolved Hide resolved tests/test_future_py3.py Outdated
@@ -2,6 +2,7 @@

PY2 = sys.version_info[0] == 2
PY3 = sys.version_info[0] == 3
GT_PY34 = sys.version_info[0:2] > (3, 4)

This comment has been minimized.

Copy link
@jodal

jodal Mar 2, 2019

Owner

Python stops the comparison of two tuples when reaching the end of the shortest:

>>> (1, 2) > (1, 1)
True
>>> (1, 2, 3) > (1, 1)
True

Thus, the slice here isn't needed:

Suggested change
GT_PY34 = sys.version_info[0:2] > (3, 4)
GT_PY34 = sys.version_info > (3, 4)
Show resolved Hide resolved pykka/future.py Outdated
Show resolved Hide resolved tests/test_future_py3.py Outdated

@lime-green lime-green force-pushed the lime-green:future-await-compat branch from d3f187b to 706d4c5 Mar 2, 2019

@lime-green lime-green force-pushed the lime-green:future-await-compat branch from 40bfce8 to 4f755e9 Mar 2, 2019

@lime-green

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2019

@jodal I addressed all your comments. You should enable builds for forked pull requests:

By default, CircleCI does not build PRs from forked repositories. To change this setting, go to the Advanced Settings of your project and set the Build forked pull requests option to On.

(https://circleci.com/docs/2.0/oss/)

@jodal

This comment has been minimized.

Copy link
Owner

commented Mar 2, 2019

Thanks for the pointer! I've turned on builds of pull requests from forks now. CircleCI is new to me as of this week, but loving it so far :-)

@jodal jodal merged commit d1eb73e into jodal:develop Mar 2, 2019

@lime-green

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2019

@jodal Yeah! I’ve also used it a little bit and I was quite impressed with it as well :) Thanks for all your work on this library!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.