Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Revert "Fix python lib install path (#207)" #236

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

sea-bass
Copy link
Collaborator

@sea-bass sea-bass commented Jan 11, 2025

This reverts commit 4e92cc4.

Turns out I took #207 in good faith, but it does cause problems.

Without that PR, this is what a custom module named admittance_params.py installs into. This is correct:

image

With that PR, it goes to the wrong folder one level up:

image

@sea-bass
Copy link
Collaborator Author

@agonzat FYI, I think this change needs to be reverted. If you have any information on the motivation behind your original PR (#207), please let us know!

Copy link
Collaborator

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

LGTM, because it seems there were less issues before the change

Copy link
Contributor

@MarqRazz MarqRazz left a comment

Choose a reason for hiding this comment

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

I just checked with my external package that uses generate_param and the generate python file is still being placed in the directory install/my_package/lib/python3.10/site-packages/my_package/generated_parameter_file.py, but running my test with colcon test still fail to import it.

To test this out I purged the binary install and checked out this branch and rebuilt with colcon build.

@sea-bass
Copy link
Collaborator Author

Weird. Did you also rebuild the generate_parameter_library packages?

@MarqRazz
Copy link
Contributor

Weird. Did you also rebuild the generate_parameter_library packages?

Yes I did. Sorry I corrected my post. It looks like that is the directory you are expecting this PR to place the files into but running test still fail so I'm not sure if you expected this to fix it being found when running colcon test.

@sea-bass
Copy link
Collaborator Author

sea-bass commented Jan 13, 2025

running test still fail so I'm not sure if you expected this to fix it being found when running colcon test.

Right, this one just reverts the install path to fix it. But the tests should still fail.

The tests will pass if you try with #237... AND you remove the __init__.py from your custom params package's source folder.

@MarqRazz
Copy link
Contributor

MarqRazz commented Jan 13, 2025

Also, the colcon test will fail unless you try with #237...

AND you remove the __init__.py from your custom params package's source folder

Sorry I missed that second PR. Yes running with #237 and removing __init__.py from my custom parameters package does fix the test! Thanks for the help @sea-bass!

@sea-bass sea-bass merged commit c9f936d into main Jan 13, 2025
7 checks passed
@sea-bass sea-bass deleted the revert-python-lib-install-change branch January 13, 2025 16:54
@imcelroy
Copy link

imcelroy commented Jan 14, 2025

@sea-bass I'm not sure this was problematic. The unit tests do run correctly now, but I get errors when I build using --merge-install and try running the test_node after these changes.

Steps to reproduce:

  1. colcon build --merge-install
  2. source install/setup.bash
  3. ros2 run generate_parameter_module_example test_node
Traceback (most recent call last):
  File "/home/user/test_ws/install/lib/generate_parameter_module_example/test_node", line 33, in <module>
    sys.exit(load_entry_point('generate-parameter-module-example==0.4.0', 'console_scripts', 'test_node')())
  File "/home/user/test_ws/install/lib/generate_parameter_module_example/test_node", line 25, in importlib_load_entry_point
    return next(matches).load()
  File "/usr/lib/python3.10/importlib/metadata/__init__.py", line 171, in load
    module = import_module(match.group('module'))
  File "/usr/lib/python3.10/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1050, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1004, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'generate_parameter_module_example.minimal_publisher'
[ros2run]: Process exited with failure 1

Here's my PYTHONPATH in order:

  • [1] /home/user/test_ws/install/generate_parameter_module_example/lib/python3.10/site-packages
  • [2] /home/user/test_ws/install/local/lib/python3.10/dist-packages
  • [3] /home/user/test_ws/install/lib/python3.10/site-packages
  • [4] /opt/ros/humble/lib/python3.10/site-packages
  • [5] /opt/ros/humble/local/lib/python3.10/dist-packages

Here's my file structure:
build/
├── generate_parameter_module_example
│   ├── build
│   │   └── lib
│   │   └── generate_parameter_module_example
│   │   ├── custom_validation.py
│   │   └── minimal_publisher.py <------ no init.py in this folder, therefore not visible to Python ?
│   ├── colcon_build.rc
│   ├── colcon_command_prefix_setup_py.sh
│   ├── colcon_command_prefix_setup_py.sh.env
│   ├── generate_parameter_module_example
│   │   ├── admittance_parameters.py <------ Here there is an init.py, so it can be found, but not in python path
│   │   └── init.py

install/
├── generate_parameter_module_example
│   └── lib
│   │ └── python3.10
│   │ │ └── site-packages <------ [1] /!\ Fails here because it doesn't find minimal_publisher.py, only param module
│   │ │ │ └── generate_parameter_module_example
│   │ │ │ │ ├── admittance_parameters.py
│   │ │ │ │ ├── init.py
├── lib
│   ├── generate_parameter_module_example
│   │   └── test_node
│   └── python3.10
│   │  └── site-packages <------ [3] missing init.py and parameter module
│   │ │  ├── generate_parameter_module_example
│   │   │  │   ├── custom_validation.py
│   │   │  │   ├── minimal_publisher.py
├── local
│   └── lib
│   └── python3.10
│   └── dist-packages
│   ├── cmake_generate_parameter_module_example <------ [2] this has everything, but wrong package
│   │   ├── admittance_parameters.py
│   │   ├── custom_validation.py
│   │   ├── init.py
│   │   ├── minimal_publisher.py
│   │   ├── parameters.yaml

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants