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 --skip-pip options #412

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

k-okada
Copy link
Contributor

@k-okada k-okada commented Nov 20, 2016

I now that having pip rosdep entory in the package to be released is not a good manner, but just in case that we have such package and still would like to release them, I think this option reduces maintenance costs very much.

@tfoote
Copy link
Member

tfoote commented Nov 20, 2016

Can you explain a little bit more about your use case? Do you expect people to pip install the dependencies separately? Can you just release python package as a ROS package? Is there a different version of the pip dependencies available from apt? Is there a graceful degradation of the packages that have this dependency if it's not present? Is it a python only package, could you release it via pip?

This is certainly the simplest solution, but I wonder if there's a clearer way to declare specific dependencies as optional. Since if we just have this option I suspect that people will just start passing this option if they run into an error due to having a pip dependency and then people will no longer be able to reliably run things that are downloaded and installed. I think it's important that we make sure that things packaged are self contained and work out of the box from the debian packages. Requiring parallel installation from a separate toolchain will lead to many more headaches as versions change going forward. We've run into pip installations getting out of date and breaking systems when users have installed tools via pip. In debs you have the option for version dependencies but that breaks when you cross package managers, and require user action out of band.

@k-okada
Copy link
Contributor Author

k-okada commented Nov 21, 2016

Find
https://github.com/jsk-ros-pkg/jsk_recognition/blob/master/jsk_perception/package.xml
for the use case of this feature, we have a ROS package that uses both C++
/ Python and some of the python code depends on pip package, and that pip
package is not released on any of version with apt system. chainer/fcn is
the deeplarning codes so may be one of the highlight or cutting/edge
feature of this package. I understand that it is not a good habit to have
so many nodes in one package, but for me this is the best way to keep code
maintained because if someone have interested in one node in the package,
they also have to care about other nodes in order to compile or use a not
that he/she first interested in. Or they have more chance to notice that
relevant nodes, In the same way I like to put as many package as possible
into a single repository. So hope you to understand this discussion may
depends on something that is not a good manner.

I'm expecting

  • beginners will use basic nodes in the deb package, C++ code and python
    with apt
  • If you become expert, and understand how to compile from sources, then
    download a package and use rosdep to install pip package,s so you can run
    all nodes in the package.
  • I'm hoping that we can use dep package and install pip manually to use
    all features of the package, but not tested yet

I already had many "headaches" on pip dependencies such as
ros-infrastructure/rosdep#482, so I agree on your
comment and concerns, other solution may develop a way to release pip
package into ROS-apt package, within git-bloom-config process, so if you
want to release package that depends on pip, they'll try to release pip
package into ROS deb packages.
ros-infrastructure/rosdep#482

◉ Kei Okada

2016-11-21 8:16 GMT+09:00 Tully Foote [email protected]:

Can you explain a little bit more about your use case? Do you expect
people to pip install the dependencies separately? Can you just release
python package as a ROS package? Is there a different version of the pip
dependencies available from apt? Is there a graceful degradation of the
packages that have this dependency if it's not present? Is it a python only
package, could you release it via pip?

This is certainly the simplest solution, but I wonder if there's a clearer
way to declare specific dependencies as optional. Since if we just have
this option I suspect that people will just start passing this option if
they run into an error due to having a pip dependency and then people will
no longer be able to reliably run things that are downloaded and installed.
I think it's important that we make sure that things packaged are self
contained and work out of the box from the debian packages. Requiring
parallel installation from a separate toolchain will lead to many more
headaches as versions change going forward. We've run into pip
installations getting out of date and breaking systems when users have
installed tools via pip. In debs you have the option for version
dependencies but that breaks when you cross package managers, and require
user action out of band.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#412 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAeG3Na6-OVPS_QSLObVut2ppFRXZTWuks5rANTggaJpZM4K3dL4
.

Copy link
Contributor

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

I guess this would work, though I'm dubious about if having unresolved pip dependencies is a good idea. If it doesn't break anything else in the toolchain then it might be fine for expert use cases like you've described.

@tfoote @dirk-thomas what do you guys think?

# L.99(def resolve_more_for_os(rosdep_key, view, installer,
# os_name, os_version):) in bloom/generators/common.py ?
warning("Key '{0}' resolved to '{1}' with installer '{2}' for os '{3}' '{4}', "
"with 'BLOOM_SKIP_PIP' environment raviable, we intentinally skip this rules"
Copy link
Contributor

Choose a reason for hiding this comment

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

'variable'

Copy link
Contributor

Choose a reason for hiding this comment

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

"but with the 'BLOOM_SKIP_PIP' environment variable set, this key is intentionally skipped."

if 'BLOOM_SKIP_PIP' in environ and environ['BLOOM_SKIP_PIP'] == '1' and inst_key == 'pip':
# force set rule to null for pip, we can escape pip in
# L.99(def resolve_more_for_os(rosdep_key, view, installer,
# os_name, os_version):) in bloom/generators/common.py ?
Copy link
Contributor

Choose a reason for hiding this comment

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

Either we can or we cannot, but this comment should be removed before merging.

@k-okada
Copy link
Contributor Author

k-okada commented Jan 28, 2017 via email

@tfoote
Copy link
Member

tfoote commented Mar 10, 2017

Thinking about this a little bit more. I'm still pretty strongly against having bloom just drop the pip dependencies. I think that if that's what you want to package then a patch could be added in the release repository. But the debian control file and the package.xml should be consistent. So that if you run rosdep on a binary installed package it will report everything satisfied.

The general approach that most users use apt and power users rosdep install things works for most use cases with this method. But breaks the expectations of any regular user who adds one of your packages to their workspace to try it out and then follows standard procedures. They can end up with pip installations they weren't expecting and don't know how to clean up.

I would much prefer if you looked into packaging the python library. @asmodehn has added some tooling for pure python packages recently. But you could also create some basic CMake wrappers that call setup.py appropriately. If the python packages are released then all your users will benefit, even the power ones.

@mikepurvis
Copy link
Contributor

I'll just chime in here that I've been experimenting with building plain python packages as part of a regular catkin workspace using a catkin_tools plugin. It's very alpha at this point (see the issues list), but give it a shot if you like:

https://github.com/mikepurvis/catkin_tools_python

Building a plain python package in-workspace like this should overlay a pip or deb-installed version of the same, but will otherwise behave the same as if the package is part of the underlying system. If it's a repo you control you can even include your own package.xml file rather than use the supplied script to generate one.

@k-okada
Copy link
Contributor Author

k-okada commented Mar 11, 2017

@tfoote @mikeferguson Thanks for comment,

@asmodehn has added some tooling for pure python packages recently

Are you referring https://github.com/asmodehn/catkin_pip ? I'll look into this with https://github.com/mikepurvis/catkin_tools_python.

The situation/challenge for me is, how to integrate python developer in to ROS ecosystem, specially we had a lot of deep learning related library written in python (https://pypi.python.org/pypi/pytorch, https://pypi.python.org/pypi/chainer, https://pypi.python.org/pypi/chainercv) and I'm thinking how to integrate student who start with these library, in to robotics research. They loves python development ecosystem, so it seems hard for them to use ROS-development systems, but we as ROS users, would like to use their results on our ROS development systems easily.

So for me, if there is an bloom-tools that can package 3rd python package would be great, like we do for cmake package with https://wiki.ros.org/bloom/Tutorials/ReleaseThirdParty method.

But I still have concern that their python package has dependency on other pip library, which may differ from Ubuntu-default python library and mixing environment with pip and ubuntu has a lot of troubles, so another idea for me is to extend roslaunch/rosrun to run package level pyenv , before they run python node which depend on other pip library.

@asmodehn
Copy link
Contributor

@k-okada My usecase for catkin_pip seems similar to yours :

  • I need to package a bunch of web related python libraries (so rosdep can find them), and I want to minimize the work.
  • I want to use uptodate python tools and libraries (eventhough I'm still stuck with 2.7)
  • I want to follow a python workflow, because all the existing python tools (and docs) assume it.
  • I want to customize ROS stuff in ThirdPartyRelease repo, and directly work with python repo.
  • I want to rely on existing OSRF infrastructure (catkin-bloom-debs)

I'm currently working like that, so think it will do the job fine for you. Please do leave issues if it doesn't. Example : https://github.com/asmodehn/pyros-config released in ros with https://github.com/asmodehn/pyros-config-rosrelease

But there are many incompatibilities between ROS and python workflows, and catkin_pip only takes care of building and packaging.

I have other packages in development (https://github.com/asmodehn/pyros and other related repos) to help during development and follow the "pythonic way". Just let me know if you want to discuss these.

@k-okada
Copy link
Contributor Author

k-okada commented Mar 14, 2017

@asmodehn Thanks for comment, and as I already send some PRs, this is (mostly) exactly what I need.
I would like to add few more thing to your usecase.

  • we're using pip package that depneds on other pip package, but that package may conflict with system python package.
  • we want to encapsulate pip installed packages, not to pollute system installed python package, when installed from deb.

I'm personally not familiar with "pythonic way" development, but does

$ source /opt/ros/indigo/setup.bash
$ cd existing_catkin_ws
$ catkin_make
$ source devel/setup.bash
$ rosrun my_package activate
$ (my_package_env) python -m my_package
$ (my_package_env) nosetests my_package
$ rosrun my_package deactivate
$

seems ok to them? and if we can write something like

<node pkg="my_package" name="my_program" type="my_program" launch-prefix="my_package_python_env" />

Then, it's perfect for me.

I have sent very primitive PR to pyros-dev/catkin_pip#98

@asmodehn
Copy link
Contributor

@k-okada It seems you are following the same process than what I did at first : trying to encapsulate what you don't know (python package) inside what you know (ros package).
I have a few comments about that :

  • Having multiple python packages into a debian package somehow violates the policy (https://www.debian.org/doc/packaging-manuals/python-policy/ch-module_packages.html#s-dependencies)
  • The debian package management system and the pip package management system have flat structure ( a package exists only in one version, and all packages have to use that version. ). Embedding multiple pip packages in one debian would introduce a hierarchy, which is not what the package system was intended for. Other package managers support hierarchical structure, and we should look into them if it is what we need...
  • Do not assume a virtualenv is just a set of files. By default a virtual environment is not relocatable
  • Mixing virtual environment and PYTHONPATH is probably not going to work the way you expect, ie. the order of path in sys.path will probably not be the one you want, and you might get some conflicts. That is because the way PYTHONPATH is used in ROS is not a usual usecase for python...
  • python workflow is much simpler than ROS or even C++. No environment variable to set or file to source, nothing to compile. Compared to ROS or C+ everything is ready out of the (virtualenv) box to start using :
alexv@alexv-pc:~$ mkvirtualenv workflow_sample
[...]
(workflow_sample) alexv@alexv-pc:~$ pip install requests
[...]
Successfully installed requests-2.13.0
(workflow_sample) alexv@alexv-pc:~$ python -c "import requests; print requests.get(\"https://httpbin.org/ip\").json()"
{u'origin': u'106.240.238.35'}

Putting all python packages inside one ros package just adds another layer of complexity on top of workspaces stack (which is already more complex than usual python workflow).
Some people have done it (check https://labs.spotify.com/2013/10/10/packaging-in-your-packaging-dh-virtualenv/), but that would require changes into the OSRF infrastructure, so after spending a few months, on and off, trying to adapt that to ROS, I now would argue it is not the best strategy. Too complex for very little gain.

Until ROS has a better integration with python (integrated interpreter + custom ROS PyPI for example), the simplest way is to match ROS way of doing things :

  • you need to make a ROS package for each python dependency of your python package. It s is a bit of work, but with catkin_pip you don't need to modify the package, so no need to fork anything. Just add a CMakeLists.txt and package.xml as a patch in a third-party release. example : https://github.com/asmodehn/webargs-rosrelease
  • you need to declare these dependencies in your package.xml
  • users will download and install package and dependencies just like any other ROS package, with rosdep.

I wrote a small doc about that in the catkin_pip documentation

This way :

  • you re using the pip package (but encapsulated as a deb)
  • you re using all dependencies as ROS packages, so the system is not polluted by global pip packages
  • as far as bloom is concerned it is just a normal package like any other.
  • you are using the regular ros way :
$ source /opt/ros/indigo/setup.bash
$ cd existing_catkin_ws
$ catkin_make
$ source devel/setup.bash
$ rosrun my_package my_script
$

To be able to develop using the "pythonic way" more things are needed (pyros-*). But that is independent of bloom and the problem for dependencies, so we can talk about it in another place.

@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

k-okada added a commit to k-okada/ros-deb-builder-action that referenced this pull request Aug 16, 2024
…, this will solve the problem such as:

The following packages have unmet dependencies:
 ros-one-jsk-data : Depends: gdown but it is not installable

where gdown is defined as pip package. See https://github.com/ros/rosdistro/blob/0adeee9a8a0fd2825b47aeeb3efeffba3465774c/rosdep/python.yaml#L1667-L1670
k-okada added a commit to k-okada/ros-deb-builder-action that referenced this pull request Aug 16, 2024
…, this will solve the problem such as:

The following packages have unmet dependencies:
 ros-one-jsk-data : Depends: gdown but it is not installable

where gdown is defined as pip package. See https://github.com/ros/rosdistro/blob/0adeee9a8a0fd2825b47aeeb3efeffba3465774c/rosdep/python.yaml#L1667-L1670
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.

6 participants