pip packages names should not use underscores

[mathias-luedtke]

While investigating ros-industrial/industrial_ci#282, I have discovered that pip does not use underscores in package names (they are discouraged in PEP8).
While pip install face_recognition works, pip freeze lists it as face-recognition==VERSION.

The latter is used by rosdep to detect the package.
(With pip 10 the detection works because rosdep uses pip show as fallback.)

[mathias-luedtke]

The same might apply for dots (e.g. backports.ssl_match_hostname) (ros-infrastructure/rosdep#420)
update: it gets listed as backports.ssl-match-hostname, underscore gets replace, but dot is kept.

[tfoote]

Your title does not match what the PEP8 link says. "Modules should have short, all-lowercase names. Underscores can be used in the module name if it improves readability. Python packages should also have short, all-lowercase names, although the use of underscores is discouraged." Discouraged vs should are different.

What action do you want to suggest based on this? This repository does not provide any modules or packages like face recognition.

[mikaelarguedas]

What action do you want to suggest based on this? This repository does not provide any modules or packages like face recognition.

I think @ipa-mdl is referring to this recently added pip package (that contains an underscore):

rosdistro/rosdep/python.yaml

Lines 1010 to 1022 in fbacef4

	python-face-recognition-pip:
	arch:
	pip:
	packages: [face_recognition]
	debian:
	pip:
	packages: [face_recognition]
	fedora:
	pip:
	packages: [face_recognition]
	ubuntu:
	pip:
	packages: [face_recognition]

[mathias-luedtke]

I am sorry, my description lacks some context..

PyPi/pip references packages with underscores by substituting these with hyphens internally.
(PEP8 might be an explanation for doing so, but I have no proper other source)

Some pip packages (like face_recognition) listed in rosdep/python.yaml still contain the underscores.
This leads to problems with rosdep, because these can get installed (pip seems to convert the names), but not detected afterwards (only hyphen versions listed).

What action do you want to suggest based on this?

I see two options:

1. patch rosdep to handle this
2. migrate ALL pip package names (packages in pip entries) to hyphen-only versions.

I would suggest to go for option 2 as it reflects the way packages are named in pip and make this an additional rule for pip packages.

[mathias-luedtke]

Another example: catkin_pkg is actually named catkin-pkg in pip:

$ pip show catkin_pkg

Name: catkin-pkg
Version: 0.4.3
Summary: catkin package library
Home-page: http://wiki.ros.org/catkin_pkg
Author: Dirk Thomas
Author-email: dthomas@osrfoundation.org
License: BSD
Location: /usr/lib/python2.7/dist-packages
Requires: argparse, docutils, python-dateutil, pyparsing

[tfoote]

Thanks for the extra context. Yeah, option 2 sounds like the right approach since the pip rules are already pip specific. And it is important that rosdep can detect them as listed. Trying to make rosdep be able to disambiguate is likely much more complicated than saying that the rosdep rule should just be what pip freeze says the package is.

[mikaelarguedas]

I see two options:
patch rosdep to handle this
migrate ALL pip package names (packages in pip entries) to hyphen-only versions.
I would suggest to go for option 2 as it reflects the way packages are named in pip and make this an additional rule for pip packages.

Seems sensible to me, please see #18126 for the first part.
For the second part: The rosdistro checks should be updated to catch and error if pip rules containing underscores in package names are submitted

[tfoote]

Yeah, I'd like to actually get to the point that we're checking package manager databases that rules are resolvable, not just formated right. I started work on the shorter_rosdep branch https://github.com/ros/rosdistro/tree/shorter_rosdep to only check changed rosdeps. After that's in we can both check formatting and actually query the database that the key exists and is potentially installable.

[clalancette]

Since we closed out #18126, I'm going to close this out as well. Feel free to reopen if you think there is still something we should do here.