Open3D
Open3D copied to clipboard
fix TypeError in make_fragments.py due to change in openo3d.io.write_…
fix TypeError in make_fragments.py due to change in openo3d.io.write_point_cloud() as of version 0.18
bug fix in make_fragments.py, as described in https://github.com/isl-org/Open3D/issues/6646
- [x ] Bug fix (non-breaking change which fixes an issue): Fixes #
- [ ] New feature (non-breaking change which adds functionality). Resolves #
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #
Motivation and Context
The current version (0.18) has this function definition:
open3d.io.write_point_cloud(filename, pointcloud, format='auto', write_ascii=False, compressed=False, print_progress=False)
but an older version (0.17) has a different function definition:
open3d.io.write_point_cloud(filename, pointcloud, write_ascii=False, compressed=False, print_progress=False)
Thus, in examples\python\reconstruction_system\make_fragments.py, the code o3d.io.write_point_cloud(pcd_name, pcd, False, True) throws the TypeError I reported above, since there is no format argument.
I changed the code to o3d.io.write_point_cloud(pcd_name, pcd, format='auto', write_ascii=False, compressed=False, print_progress=True) and now it runs without the TypeError
Checklist:
- [ ] I have run
python util/check_style.py --applyto apply Open3D code style to my code. - [ ] This PR changes Open3D behavior or adds new functionality.
- [ ] Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is updated accordingly.
- [ ] I have added or updated C++ and / or Python unit tests OR included test results (e.g. screenshots or numbers) here.
- [ ] I will follow up and update the code if CI fails.
- [ ] For fork PRs, I have selected Allow edits from maintainers.
Description
(sorry, this is my first ever pull request; thankfully, it is a very simple change)
Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.
@hexaflexa Thank you for this contribution! ~~Please apply the style as described in the PR checklist so the PR passes CI. Also, would you please check for other uses of write_point_cloud in examples and verify that the one make_fragments.py is the only one that needed to be fixed? Thanks.~~
@hexaflexa We just discussed this PR internally and we think that the real issue is that the new format parameter should have been placed at the end of the parameter list in order to maintain compatibility with existing code. This PR fixes the broken Open3D sample but we are worried about user code that the placement of the format parameter will break.
Would you be willing to change the signature of write_point_cloud in this PR instead of changing the example? You would only need to change the pybind for it here: https://github.com/isl-org/Open3D/blob/main/cpp/pybind/io/class_io.cpp#L205.
Thanks!
Hi @errissa,
I am not an expert in the Open3D code (only discovered it a few days ago). But would changing the python signature of write_point_cloud make it inconsistent with the order in the C++ signatures for ReadPointCloud, ReadPointCloudOption, WritePointCloud, and WritePointCloudOption, where format comes before the other boolean options?
And then the pybind for read_point_cloud would also have to be changed for consistency with write_point_cloud, but both would be different from the C++ order.