Skip to content
This repository was archived by the owner on Nov 23, 2017. It is now read-only.

Add get_future() #479

Closed
wants to merge 1 commit into from
Closed

Add get_future() #479

wants to merge 1 commit into from

Conversation

AraHaan
Copy link

@AraHaan AraHaan commented Dec 30, 2016

A nice function that gets and returns the future that is much more
easier to manage without users of asyncio having to hack duck type it
just to see if ensure_future exists or not. Bad for coverage with
packages like aiohttp which operates on a future that is unsure and bugs
coverage services like Codecov. With this they are for sure to get the
correct future for their version of asyncio.

Note: Will update this PR if and builds on this fails to ensure the function works as intended.

A nice function that gets and returns the future that is much more
easier to manage without users of asyncio having to hack duck type it
just to see if ensure_future exists or not. Bad for coverage with
packages like aiohttp which operates on a future that is unsure and bugs
coverage services like Codecov. With this they are for sure to get the
correct future for their version of asyncio.
@1st1
Copy link
Member

1st1 commented Dec 30, 2016

I'm not sure I see any utility in the proposed get_future function. ensure_future has been available since 3.5 and asyncio.async() throws a deprecation warning. At this point of time, we don't need any wrappers around those functions.

@AraHaan
Copy link
Author

AraHaan commented Dec 30, 2016

The issue is because of this mostly: https://codecov.io/gh/KeepSafe/aiohttp/compare/8469b6dea75c17732fd4de5ef3a1846cadf220cb...3dc08a056c0b7fa1d3d2f8ec2b53e9fb60646bec

Many packages which also supports Python 3.4 has this problem with getting the future on things like codecov.io that checks their packages for coverage.

@gvanrossum
Copy link
Member

This doesn't make sense to me. You're adding this to the file containing ensure_future() so the hasattr() check will always succeed, so you're just introducing a new function that apps should be testing for.

@gvanrossum gvanrossum closed this Dec 30, 2016
@AraHaan
Copy link
Author

AraHaan commented Dec 30, 2016

Take a look at the URL provided. It is a great workaround if it was Added for not only 3.4 but also for 3.5+

@gvanrossum
Copy link
Member

That link is a 404 for me.

@AraHaan
Copy link
Author

AraHaan commented Dec 30, 2016

Fixed it. the reason why putting it inside of aiohttp is bad is because it jacks with the PR making it's check fail. aio-libs/aiohttp#1521.

@gvanrossum
Copy link
Member

The fixed link works, but nothing will add that function to the stdlib for Python versions that don't have it, so people will still write the try/except. I stand by my opinion that this is the wrong way to fix it.

@AraHaan
Copy link
Author

AraHaan commented Dec 30, 2016

hmm unless one can go in and do

try:
    ensure_future = asyncio.ascync
except DeprecationWarning:
    ensure_future = asyncio.ensure_future

But I am sure the DeprecationWarning will only trigger when it is called.

@gvanrossum
Copy link
Member

It makes sense to add that to the user code (or even to libraries that depend on asyncio). But it doesn't make sense to add that to asyncio itself.

@Martiusweb
Copy link
Member

Martiusweb commented Dec 30, 2016

@AraHaan Shouldn't it be the reverse?

Something like:

ensure_future = getattr(asyncio, 'ensure_future', asyncio.async)

@AraHaan
Copy link
Author

AraHaan commented Dec 30, 2016

oh yeah

@gvanrossum
Copy link
Member

For maximum future-proofing I'd recommend

ensure_future = getattr(asyncio, 'ensure_future', getattr(asyncio, 'async', None))

This will work in the distant future when asyncio.async no longer exists at all, and even when async becomes a keyword.

@AraHaan
Copy link
Author

AraHaan commented Dec 30, 2016

ok, also I noticed a problem with distutils which was closed as not a distutils problem but if they read the actual setuptools and distrubute source codes they would find they both use distutils for their setup function so it really is a distutils issue more info on that here: pypa/setuptools#906
please have a look at this @gvanrossum there is literally no other way I can bypass the issue with bugtrack_url other than editing the package online on pypi.python.org which is sadly not letting me do it anymore for any package as it requires me to upload a PKG-INFO file that sadly setuptools and distribute strips all because it barrows the distutils.core.setup function when it exports setup by doing:

setup = distutils.core.setup

The line in question on setuptools is here: https://github.com/pypa/setuptools/blob/master/setuptools/__init__.py#L112

But yeah my current idea for the solution is either get them to update distutils to work on that for 3.7, 3.6.1, and 3.5.3 or to have setuptools steal the setup function from distutils only to monkey patch it to accept bugtrack_url.

The bugtrack issue that was closed: http://bugs.python.org/issue29115

@python python locked and limited conversation to collaborators Dec 30, 2016
@1st1
Copy link
Member

1st1 commented Dec 30, 2016

I'm locking this conversation. Please report bugs for other projects on their issue trackers.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants