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

Add missing pyyaml dependency #150

Merged
merged 1 commit into from
Jul 15, 2019

Conversation

rotu
Copy link
Contributor

@rotu rotu commented Jul 13, 2019

ament_clang_format will crash without this dependency with something like:

➜ ament_clang_format
Traceback (most recent call last):
  File "/Users/dan/temp/env/bin/ament_clang_format", line 11, in <module>
    load_entry_point('ament-clang-format', 'console_scripts', 'ament_clang_format')()
  File "/Users/dan/temp/env/lib/python3.7/site-packages/pkg_resources/__init__.py", line 489, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/Users/dan/temp/env/lib/python3.7/site-packages/pkg_resources/__init__.py", line 2793, in load_entry_point
    return ep.load()
  File "/Users/dan/temp/env/lib/python3.7/site-packages/pkg_resources/__init__.py", line 2411, in load
    return self.resolve()
  File "/Users/dan/temp/env/lib/python3.7/site-packages/pkg_resources/__init__.py", line 2417, in resolve
    module = __import__(self.module_name, fromlist=['__name__'], level=0)
  File "/Users/dan/temp/ament_lint/ament_clang_format/ament_clang_format/main.py", line 26, in <module>
    import yaml
ModuleNotFoundError: No module named 'yaml'

Signed-off-by: Dan Rose <dan@digilabs.io>
Copy link
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.

LGTM, thanks!

Linux: Build Status

@jacobperron jacobperron merged commit 40cf85e into ament:master Jul 15, 2019
@dirk-thomas
Copy link
Contributor

The dependency needs to be declared in the package manifest.

@rotu
Copy link
Contributor Author

rotu commented Jul 30, 2019

The dependency needs to be declared in the package manifest.

Oops. I figured putting it in setup.py was good enough, since typically setuptools makes sure dependency packages get installed.

What's the relation between the package manifest and the setup.py for Python package dependencies? Is there a way to declare it in one place for DRYer code?

@dirk-thomas
Copy link
Contributor

What's the relation between the package manifest and the setup.py for Python package dependencies?

The dependencies declared in the manifest are used to determine the topological build order as well as during packaging are mapped to e.g. Debian package names.

Is there a way to declare it in one place for DRYer code?

No.

rotu added a commit to RoverRobotics-forks/ament_lint that referenced this pull request Jul 30, 2019
as per ament#150 (comment)

Signed-off-by: Dan Rose <dan@digilabs.io>
@rotu rotu mentioned this pull request Jul 30, 2019
@rotu
Copy link
Contributor Author

rotu commented Jul 30, 2019

The dependencies declared in the manifest are used to determine the topological build order as well as during packaging are mapped to e.g. Debian package names.

Hmmm. That seems problematic for a multiple-interpreter system. If you have your workspace building on top of python3.7 but apt install python3-yaml installs in python3.6, your workspace won't have the dependency at runtime. Why not just get it from PyPi?

@dirk-thomas
Copy link
Contributor

dirk-thomas commented Jul 30, 2019

If you have your workspace building on top of python3.7 but apt install python3-yaml installs in python3.6

If you use a non-default Python interpreter on your system the Debian packages will likely not help you much since they are standardized on the default version shipped in that distribution. In that case you do whatever you like to satisfy your dependencies (e.g. a venv).

Why not just get it from PyPi?

Because a Debian package can't depend on a package from PyPi - it must depend on Debian packages.

@rotu
Copy link
Contributor Author

rotu commented Jul 30, 2019

If you use a non-default Python interpreter on your system the Debian packages will likely not help you much since they are standardized on the default version shipped in that distribution.

So why doesn't colcon install the dependencies from PyPI as per the setup.py documentation? Colcon knows the current interpreter and the current install location. Presumably pip works on Debian - why use python packaging tools but non-standard python dependency resolution?

Edit: found useful discussion here ros/rosdistro#18840

@dirk-thomas
Copy link
Contributor

So why doesn't colcon install the dependencies

colcon intentionally doesn't install any dependencies. It is explicitly listed as a non-goal in the docs. Why? Because the topic is so complex on its own that it is separated into a different tool: rosdep.

Presumably pip works on Debian

There are severe downsides of using pip to install Python packages if there is an equivalent Debian package. E.g. the pip installed package overlay your Debian package. So if in the future your package manager is automatically updating your Debian packages you might have an incompatible version from pip breaking things.

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.

None yet

3 participants