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

Remove deprecated use of asyncio.coroutine decorator. #64

Merged
merged 1 commit into from
Jun 23, 2020

Conversation

clalancette
Copy link
Contributor

In later versions of Python, @asyncio.coroutine is deprecated
in favor of "async def". However, in order to support python
back to 3.6, we also have to switch from using "yield from"
to instead using "await".

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

Note that according to the Python documentation, this should work all the way back to Python 3.5. But this definitively breaks support for Python 2, so I've removed it from Travis.

@clalancette clalancette requested a review from wjwwood June 19, 2020 14:27
@dirk-thomas
Copy link
Contributor

This resolves #61.

@dirk-thomas
Copy link
Contributor

dirk-thomas commented Jun 19, 2020

Dropping Python 2 and 3.5 3.4 support needs to be reflected in the stdeb.cfg file. It probably makes sense to create a branch from which potential patch releases for those versions can be made. Afterwards the minor should be bumped to leave room for such patch releases.

@clalancette
Copy link
Contributor Author

Dropping Python 2 and 3.5 support needs to be reflected in the stdeb.cfg file.

Just to be clear, this will only drop support for 3.4 (3.5 should still work, though I haven't tested it there).

As far as the stdeb.cfg file goes, I'll definitely go ahead and change the X-Python3-Version to reflect the new reality. I'll also remove the "Depends" and "Suite", just leaving "Depends3" and "Suite3" in place. Was there anything else you were thinking of that needed to be changed in stdeb.cfg?

It probably makes sense to create a branch from which potential patch releases for those versions can be made. Afterwards the minor should be bumped to leave room for such patch releases.

I'll wait for @wjwwood to weigh in here, but that sounds like a good approach to me.

@dirk-thomas
Copy link
Contributor

Was there anything else you were thinking of that needed to be changed in stdeb.cfg?

Yes, Python 2 releases must be blocked all together: No-Python2:

@clalancette
Copy link
Contributor Author

Yes, Python 2 releases must be blocked all together: No-Python2:

Hm, I don't see the directive documented anywhere, and it doesn't look like it is in the list at https://github.com/astraw/stdeb/blob/57dd69cf652377af52ba307ea29c8d553e1f7b52/stdeb/util.py#L130 .

@clalancette
Copy link
Contributor Author

#65 should fix the failing CI.

@wjwwood
Copy link
Member

wjwwood commented Jun 19, 2020

Why does this break support for Python 2? yield from also does not work in Python 2, this package is specially set up to handle that...

I mean I'm not really attached to the Python 2 support any longer since catkin tools went python 3 only, but I'm just saying, it shouldn't be required.

@clalancette
Copy link
Contributor Author

Why does this break support for Python 2? yield from also does not work in Python 2, this package is specially set up to handle that...

Oh, good point. I didn't actually realize how this was setup.

I mean I'm not really attached to the Python 2 support any longer since catkin tools went python 3 only, but I'm just saying, it shouldn't be required.

Yeah, you are right. I'll get rid of the changes to remove Python 2 support; we can do it separately.

Thanks.

In later versions of Python, @asyncio.coroutine is deprecated
in favor of "async def".  However, in order to support python
back to 3.6, we also have to switch from using "yield from"
to instead using "await".

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette clalancette force-pushed the clalancette/remove-async-coroutine branch from 90ad360 to eb12b5e Compare June 22, 2020 12:39
@dirk-thomas
Copy link
Contributor

I don't see the directive documented anywhere, and it doesn't look like it is in the list at https://github.com/astraw/stdeb/blob/57dd69cf652377af52ba307ea29c8d553e1f7b52/stdeb/util.py#L130 .

The No-PythonX option is not from stdeb but ros_release_python to determine for which Python versions to invoke stdeb.

@clalancette
Copy link
Contributor Author

The No-PythonX option is not from stdeb but ros_release_python to determine for which Python versions to invoke stdeb.

Ah, I see. Thanks for the info, that will be useful when I do go to remove Python 2 from this package.

@wjwwood wjwwood merged commit ac6adfe into master Jun 23, 2020
@wjwwood wjwwood deleted the clalancette/remove-async-coroutine branch June 23, 2020 17:13
@dirk-thomas
Copy link
Contributor

Just to be clear, this will only drop support for 3.4

The stdeb.cfg file still needs to be update to remove Debian Jessie from the list of targeted platforms since it ships with Python 3.4.

@wjwwood
Copy link
Member

wjwwood commented Jun 24, 2020

#67

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

Successfully merging this pull request may close these issues.

3 participants