add python_version key to REP 153 #207
Conversation
dirk-thomas
force-pushed
the
dirk-thomas/add-python_version-to-index
branch
from
18d67f9
to
995d6d6
Compare
Co-Authored-By: Jacob Perron <[email protected]>
Co-Authored-By: Jacob Perron <[email protected]>
| * python_version: an optional integer describing the major version of | ||
| Python of the ROS distribution. |
There was a problem hiding this comment.
Given that this is optional, should the REP define a default?
There was a problem hiding this comment.
I don't think there is any default value which would be reasonable / not have common cases where it would be wrong. The official rosdistro database is being update to define the value for every ROS distro: ros/rosdistro#22484.
There was a problem hiding this comment.
Well, given that the REP specifies that unknown keys are to be ignored, along with the fact that until very recently everything was python 2, clients will be assuming python 2 unless support for this key is added, right? In other words, it seems like the de-facto default is 2, here, but only because of assumptions made by consumers of the REP. That seems potentially problematic, but I suppose also mostly academic.
There was a problem hiding this comment.
along with the fact that until very recently everything was python 2, clients will be assuming python 2 unless support for this key is added, right?
That is incorrect. While all ROS 1 distros up to Melodic are using Python 2 all ROS 2 distributions are using Python 3.
There was a problem hiding this comment.
Great point-- that distinction is currently hard-coded then, yes? But it's "easy" right now because you can split ROS 1->Python 2, ROS 2->Python 3, until Noetic. Which comes back to this PR. Very good, thank you.
This specifies the semantic of the new key
python_versionintroduced in ros-infrastructure/rosdistro#145.