Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

fix issue 2141: raw_input does not exist in python 3#2143

Merged
jacobperron merged 2 commits intoros:noetic-develfrom
boschresearch:bugfix/python3-raw-input
Mar 24, 2021
Merged

fix issue 2141: raw_input does not exist in python 3#2143
jacobperron merged 2 commits intoros:noetic-develfrom
boschresearch:bugfix/python3-raw-input

Conversation

@bst
Copy link
Copy Markdown
Contributor

@bst bst commented Mar 8, 2021

This fixes issue #2141 by using input() instead of raw_input() and simulating python 3's `input() for python 2.

Signed-off-by: Scherer Sebastian (CR/AAS3) <Sebastian.Scherer2@de.bosch.com>
Copy link
Copy Markdown
Contributor

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Comment on lines +46 to +49
try:
input = raw_input # Python 2.x: Simulate 3.x behavior.
except NameError:
pass # Python 3.x: There is no raw_input, use input directly.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to make this backwards compatible?

As I understand, since ROS Noetic we're only expected to support Python 3 (see REP 3.

cc/ @sloretz

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, on the noetic branch there's no need for the backwards compatibility. Some people use Python 3 with older ROS distros (like arch melodic users), so mabye @bst added it in hopes for a backport to melodic?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, i wasn't aware of that. thanks for pointing that out 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I was actually not aware that we here do not need to support Python 2 anymore.

I have no preference in any direction. If you prefer I can also remove the workaround for supporting Python 2.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove the workaround here, and if there is desire to backport to Melodic, we can consider the workaround in a backport PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree. we would not want to keep something will never be used.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. I just removed the python2 compatibility workaround I had added before.

Signed-off-by: Scherer Sebastian (CR/AAS3) <Sebastian.Scherer2@de.bosch.com>
Copy link
Copy Markdown
Contributor

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants