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

fix round_collada_numbers.py by opening files in binary mode #2983

Merged
merged 2 commits into from
Dec 15, 2021

Conversation

tbazina
Copy link
Contributor

@tbazina tbazina commented Dec 7, 2021

Description

Minor change to fix round_collada_numbers.py script by opening files for reading and saving in binary mode. No reference issue.

@welcome
Copy link

welcome bot commented Dec 7, 2021

Thanks for helping in improving MoveIt and open source robotics!

@rhaschke
Copy link
Contributor

rhaschke commented Dec 7, 2021

Could you please describe the problem with the current approach? Collada files are text files (XML). So why open them in binary mode?

@tbazina
Copy link
Contributor Author

tbazina commented Dec 7, 2021

When running:
rosrun moveit_kinematics round_collada_numbers.py <file_name>.dae <file_name>_rounded.dae 5

the error traceback is:

Collada Number Rounder
Rounding numbers to 5 decimal places

Traceback (most recent call last):
  File "/opt/ros/noetic/lib/moveit_kinematics/round_collada_numbers.py", line 97, in <module>
    dom = etree.parse(io.BytesIO(xml))
TypeError: a bytes-like object is required, not 'str'

Per io documentation (https://docs.python.org/3/library/io.html#binary-i-o), "the easiest way to create a binary stream is with open() with 'b' in the mode string". After the fix, the script works fine.

@codecov
Copy link

codecov bot commented Dec 7, 2021

Codecov Report

Merging #2983 (923cc88) into master (fd36674) will increase coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2983      +/-   ##
==========================================
+ Coverage   61.55%   61.63%   +0.08%     
==========================================
  Files         370      370              
  Lines       33145    33145              
==========================================
+ Hits        20399    20425      +26     
+ Misses      12746    12720      -26     
Impacted Files Coverage Δ
...dl_kinematics_plugin/src/kdl_kinematics_plugin.cpp 78.28% <0.00%> (+0.38%) ⬆️
...on/pick_place/src/approach_and_translate_stage.cpp 74.69% <0.00%> (+0.64%) ⬆️
...ipulation/pick_place/src/manipulation_pipeline.cpp 73.56% <0.00%> (+1.66%) ⬆️
..._interface/src/detail/constrained_goal_sampler.cpp 76.48% <0.00%> (+1.97%) ⬆️
moveit_core/robot_model/src/joint_model_group.cpp 64.15% <0.00%> (+2.28%) ⬆️
...e/src/parameterization/model_based_state_space.cpp 72.68% <0.00%> (+3.11%) ⬆️
.../ompl_interface/src/detail/constrained_sampler.cpp 60.98% <0.00%> (+17.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd36674...923cc88. Read the comment docs.

- directly parse and write from/to file
- explicitly specify an encoding
Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation. I added another cleanup commit and tested on both, python2 and python3.

@tbazina
Copy link
Contributor Author

tbazina commented Dec 8, 2021

Great, thank you for the additional cleanup. Everything seems to be working now.

@rhaschke rhaschke merged commit 5ddb027 into moveit:master Dec 15, 2021
@welcome
Copy link

welcome bot commented Dec 15, 2021

Congrats on getting your first MoveIt pull request merged and improving open source robotics!

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.

2 participants