Skip to content

Optional deps humble #150

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

Merged
merged 3 commits into from
Jan 13, 2025
Merged

Optional deps humble #150

merged 3 commits into from
Jan 13, 2025

Conversation

jbarnett-bdai
Copy link
Contributor

Previously, we added version pinning by default for matplotlib and numpy, to avoid conflicts with system-installed packages in ROS2. This PR removes the default version pinning, moving to a ros-humble under project.optional-dependencies in pyproject.toml.

Now, to avoid version conflicts under ROS2 (Humble), the package can be installed via

pip install spatialmath-python[ros-humble]

and the README.md is updated to reflect this.

Tested that:

  • adding [ros-humble] to the installation command enforces ROS-compatible library versions, avoiding conflict;
  • installing without [ros-humble] installs up-to-date versions of the libraries.

@petercorke
Copy link
Collaborator

This is a great solution to the issue. Should we have a ros-XXX option for all current (or recently EOLd) ROS2 distros? The numpy and matplotlib pins they require are easy to find.

I'm guessing that the fact we're having this conversation indicates that ROS2 systems are a big use case either within BDAI or the broader user base. If that's the case should we have tooling to make it easy to convert ROS messages to spatialmath objects and vice versa?

@myeatman-bdai
Copy link
Collaborator

I'm guessing that the fact we're having this conversation indicates that ROS2 systems are a big use case either within BDAI or the broader user base.

This is most definitely the case!

If that's the case should we have tooling to make it easy to convert ROS messages to spatialmath objects and vice versa?
We have some internal tooling for this that is not very pretty. There's actually a ROS package out there for this conversion as well, https://github.com/qcr/spatialmath_ros, but we didn't know about it until recently.

I'll @jbarnett-bdai this could be a decent amount of bang for the amount of man-hour bucks put in, IMO.

@myeatman-bdai myeatman-bdai merged commit 0ff5c83 into master Jan 13, 2025
15 checks passed
@myeatman-bdai myeatman-bdai deleted the jbarnett-optional-deps-humble branch January 13, 2025 15:30
@petercorke
Copy link
Collaborator

OK, Ben was a postdoc in our lab. There's another one by a former student https://github.com/CallumJHays/spatialmath-rospy. The latter has MIT licence, Ben's one has no licence but I can ping Ben and the lab and get them to add a licence.

If we do this, I'd like to see it as a separate file, or even submodule, that doesn't cause issues if there is no ROS installed on the platform.

@gbattocletti
Copy link

Hi all,
I am trying to install the default version of the package on windows 10 + python 3.13, and I am still getting an error related to the versions of matplotlib/numpy. I tried the installation in a new conda environment by running pip install spatialmath-python but the numpy version is still being constrained to the one specified in the [ros-humble] version (i.e., <2, >=1.22), and ultimately results in an installation error.

image

I tried doing the same in an environment with python 3.10 and the installation works correctly, albeit by still forcing the versions matplotlib==3.5.1 and numpy<2. In this sense, I notice that in the build tests the python version on windows stops at 3.10 https://github.com/bdaiinstitute/spatialmath-python/actions/runs/12750545986. I'd suggest to add tests for higher python versions should be added in order to catch this type of errors.

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.

4 participants