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

Delete unported moveit_joy visualization demo #1541

Merged
merged 1 commit into from
Dec 12, 2022

Conversation

AndyZe
Copy link
Member

@AndyZe AndyZe commented Aug 28, 2022

Description

I was poking around, looking into build warnings about deprecated setup.py. While I didn't find a complete solution, I think we can safely delete all of these scripts since they still refer to catkin.

@codecov
Copy link

codecov bot commented Aug 28, 2022

Codecov Report

Base: 50.24% // Head: 51.08% // Increases project coverage by +0.85% 🎉

Coverage data is based on head (0e0a190) compared to base (ddd50ce).
Patch has no changes to coverable lines.

❗ Current head 0e0a190 differs from pull request most recent head 148587f. Consider uploading reports for the commit 148587f to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1541      +/-   ##
==========================================
+ Coverage   50.24%   51.08%   +0.85%     
==========================================
  Files         374      380       +6     
  Lines       31277    31802     +525     
==========================================
+ Hits        15712    16243     +531     
+ Misses      15565    15559       -6     
Impacted Files Coverage Δ
...ics_plugin_loader/src/kinematics_plugin_loader.cpp 69.75% <0.00%> (-13.01%) ⬇️
...ma_kinematics_plugin/src/lma_kinematics_plugin.cpp 73.46% <0.00%> (-3.90%) ⬇️
...dl_kinematics_plugin/src/kdl_kinematics_plugin.cpp 75.36% <0.00%> (-3.43%) ⬇️
...anner/src/trajectory_blender_transition_window.cpp 99.17% <0.00%> (-0.83%) ⬇️
...traint_samplers/src/constraint_sampler_manager.cpp 88.39% <0.00%> (-0.64%) ⬇️
moveit_ros/moveit_servo/src/pose_tracking.cpp 76.78% <0.00%> (-0.32%) ⬇️
moveit_core/robot_model/src/joint_model_group.cpp 64.87% <0.00%> (-0.24%) ⬇️
moveit_core/robot_state/src/robot_state.cpp 47.91% <0.00%> (-0.04%) ⬇️
moveit_core/transforms/src/transforms.cpp 86.67% <0.00%> (ø)
moveit_core/robot_model/src/link_model.cpp 100.00% <0.00%> (ø)
... and 103 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@sjahr sjahr left a comment

Choose a reason for hiding this comment

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

Good idea to address the setup.py deprecation warnings! I fear that removing these files might lead to dead code, for example the \python in moveit_core. @peterdavidfagan what do you think about this? Will the deprecation warning be addressed by your work on MoveIt2 python bindings?

moveit_commander/setup.py Outdated Show resolved Hide resolved
moveit_core/setup.py Outdated Show resolved Hide resolved
@AndyZe
Copy link
Member Author

AndyZe commented Aug 29, 2022

Small misconception here:

I fear that removing these files might lead to dead code

All of this code is already dead!

You're right, the Python code in moveit_core and all of moveit_commander should probably be removed all at once. I'll remove them from this PR and tweak the other packages as well.

@AndyZe AndyZe closed this Aug 29, 2022
@AndyZe AndyZe reopened this Aug 29, 2022
@AndyZe AndyZe changed the title Delete deprecated setup.py Delete unported moveit_joy visualization demo Aug 29, 2022
@peterdavidfagan
Copy link
Member

peterdavidfagan commented Aug 29, 2022

Hi @sjahr,

Thanks for looping me into this conversation.

I fear that removing these files might lead to dead code, for example the \python in moveit_core

I have actually removed this code in my current working branch of moveit_py (see here) so it would be ok to remove in my mind as @AndyZe has suggested. I can look to open a separate pr just for removing such code.

Will the deprecation warning be addressed by your work on MoveIt2 python bindings?

I am happy to address this as part of my GSoC project. For any of the code related to the library I am planning to release I would expect to ensure any warning including those related to deprecations to be addressed where they possibly can be. If existing code that is being maintained contains such deprecation warning I would be happy to fix these too.

@AndyZe
Copy link
Member Author

AndyZe commented Aug 29, 2022

Thanks @peterdavidfagan.

My bit of googling into this type of error suggested that it's a ROS2 build system issue and we need to wait for Open-Robotics to fix it. Nothing to be done on our end.

--- stderr: moveit_configs_utils
/usr/lib/python3/dist-packages/setuptools/command/install.py:34: SetuptoolsDeprecationWarning: setup.py install is deprecated. Use build and pip and other standards-based tools.
  warnings.warn(

@AndyZe AndyZe merged commit e23fb8f into moveit:main Dec 12, 2022
@AndyZe AndyZe deleted the andyz/deprecated_setup_py branch December 12, 2022 22:53
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