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

refactor: drop ROS 2 dependency for core rai packages #411

Merged
merged 8 commits into from
Feb 7, 2025

Conversation

maciejmajek
Copy link
Member

@maciejmajek maciejmajek commented Feb 6, 2025

Purpose

The current dependency on ROS2's colcon build system for Python packages introduces unnecessary complexity and limitations. By migrating core RAI packages to a pure Poetry-based build system, we can significantly improve package management, distribution, and development workflow.

Proposed Changes

  • Migrated black + flake8 + sort combo to ruff (previous combo was problematic, definitely not bug free)
    Refactored the following packages to remove ROS2 build system dependencies:
  • rai_core
  • rai_asr
  • rai_tts
    These packages now use Poetry exclusively for:
  • Dependency management
  • Package building
  • Version control

Issues

  • Links to relevant issues

Testing

  • How was it tested, what were the results?

@maciejmajek maciejmajek force-pushed the refactor/drop-ros2-dep branch from f2eed6e to 008252c Compare February 6, 2025 21:19
@maciejmajek
Copy link
Member Author

Tests are failing due to vm-041 problems (out of resources). Will be fixed later. The tests run fine on pc-054 which has more resources.

@rachwalk
Copy link
Contributor

rachwalk commented Feb 7, 2025

This is just a pre-review comment, but I think there is an argument to be made for updating README with this PR (I don't see it in the list of modified files)

@maciejmajek
Copy link
Member Author

This is just a pre-review comment, but I think there is an argument to be made for updating README with this PR (I don't see it in the list of modified files)

Fortunately, due to nature of these changes, there is no need for any modifications.

poetry.lock Outdated Show resolved Hide resolved
@rachwalk
Copy link
Contributor

rachwalk commented Feb 7, 2025

Does this PR affect these issues?
#286
#302
#204
#261
#387

@maciejmajek
Copy link
Member Author

Does this PR affect these issues? #286 #302 #204 #261 #387

This is just a first step for most of these issues actually.

@rachwalk
Copy link
Contributor

rachwalk commented Feb 7, 2025

Does this PR affect these issues? #286 #302 #204 #261 #387

This is just a first step for most of these issues actually.

It would be useful to mention it either in the PR then (so github can track the mention in merged PR) or in the issues explicietly. If you could either of these things for issues you deem relevant.

@maciejmajek
Copy link
Member Author

@rachwalk could you please do a clean setup of rai to check if everything works properly in your ide?

@boczekbartek boczekbartek self-requested a review February 7, 2025 14:29
Copy link
Member

@boczekbartek boczekbartek left a comment

Choose a reason for hiding this comment

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

@maciejmajek
I've built docker with this feature. I can import rai, rai_tts and rai_asr without souring of ros environment. (however I have some warnings)

robo-srv-004 ➜  rai git:(refactor/drop-ros2-dep) ✗ docker run --rm -it rai:jazzy /bin/bash                                      
root@998b62a84ae6:/rai# poetry shell
Spawning shell within /root/.cache/pypoetry/virtualenvs/rai-framework-LG7o02pr-py3.12
root@998b62a84ae6:/rai# . /root/.cache/pypoetry/virtualenvs/rai-framework-LG7o02pr-py3.12/bin/activate
(rai-framework-py3.12) root@998b62a84ae6:/rai# python
Python 3.12.3 (main, Apr 10 2024, 05:33:47) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import rai
/root/.cache/pypoetry/virtualenvs/rai-framework-LG7o02pr-py3.12/lib/python3.12/site-packages/pydub/utils.py:300: SyntaxWarning: invalid escape sequence '\('
  m = re.match('([su]([0-9]{1,2})p?) \(([0-9]{1,2}) bit\)$', token)
/root/.cache/pypoetry/virtualenvs/rai-framework-LG7o02pr-py3.12/lib/python3.12/site-packages/pydub/utils.py:301: SyntaxWarning: invalid escape sequence '\('
  m2 = re.match('([su]([0-9]{1,2})p?)( \(default\))?$', token)
/root/.cache/pypoetry/virtualenvs/rai-framework-LG7o02pr-py3.12/lib/python3.12/site-packages/pydub/utils.py:310: SyntaxWarning: invalid escape sequence '\('
  elif re.match('(flt)p?( \(default\))?$', token):
/root/.cache/pypoetry/virtualenvs/rai-framework-LG7o02pr-py3.12/lib/python3.12/site-packages/pydub/utils.py:314: SyntaxWarning: invalid escape sequence '\('
  elif re.match('(dbl)p?( \(default\))?$', token):
/root/.cache/pypoetry/virtualenvs/rai-framework-LG7o02pr-py3.12/lib/python3.12/site-packages/pydub/utils.py:170: RuntimeWarning: Couldn't find ffmpeg or avconv - defaulting to ffmpeg, but may not work
  warn("Couldn't find ffmpeg or avconv - defaulting to ffmpeg, but may not work", RuntimeWarning)
>>> import rai_tts
>>> import rai_asr

Copy link
Contributor

@rachwalk rachwalk left a comment

Choose a reason for hiding this comment

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

LGTM

@maciejmajek maciejmajek merged commit ce4885c into development Feb 7, 2025
5 checks passed
@maciejmajek maciejmajek deleted the refactor/drop-ros2-dep branch February 7, 2025 15:00
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.

Refactor ROS2 packages into python packages
3 participants