Skip to content

Conversation

@saurabheights
Copy link
Contributor

@saurabheights saurabheights commented Jun 13, 2023

Type

  • 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 integrate.py in examples/python/t_reconstruction did not work. I am using open3d v0.17.0 with python 3.10.10 on ubuntu 22.04.

  1. ~~store_true no longer works, see here. ~~ MY BAD - This is not a bug.
  2. Config file has integration_mode which can be removed in favor of integrate_color which is IMO easy to understand.
  3. Documentation is no longer synced - http://www.open3d.org/docs/release/tutorial/t_reconstruction_system/integration.html#activation
  4. integrate_color is method argument here but missing in caller

Checklist:

  • I have run python util/check_style.py --apply to 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

Screenshot from 2023-06-13 23-06-34
Screenshot from 2023-06-13 23-06-02


This change is Reviewable

@update-docs
Copy link

update-docs bot commented Jun 13, 2023

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

@saurabheights
Copy link
Contributor Author

saurabheights commented Jun 13, 2023

P.S. I couldnt test changes for documentation if the new linking (to the code) happens correctly. I need to check how to build the docs.

Update - I have tested the changes and docs are fine.

Note - I have not changed documentation of

@saurabheights
Copy link
Contributor Author

saurabheights commented Jun 14, 2023

Update - Just noticed my mistake. I forgot to set --integrate_color in pycharm configuration and assumed I had done it. I will update the PR soon. - No further changes needed. Since I have moved integrate_color from console args to config file also for reason 2 and 4, we can ignore reason 1.

@saurabheights saurabheights force-pushed the khanduja/fix_examples branch from 4aa129d to cad75ee Compare June 15, 2023 14:34
@ssheorey ssheorey requested a review from theNded June 29, 2023 19:39
Copy link
Contributor

@theNded theNded left a comment

Choose a reason for hiding this comment

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

LGTM, I will also work on improving it.

@theNded
Copy link
Contributor

theNded commented Jun 30, 2023

@ssheorey It is ready to merge, my follow-up upgrades will be in separate PRs.

@ssheorey
Copy link
Member

Thanks @saurabheights !

@ssheorey ssheorey merged commit 7dc22f2 into isl-org:master Jun 30, 2023
@saurabheights saurabheights deleted the khanduja/fix_examples branch July 2, 2023 20:29
@saurabheights saurabheights mentioned this pull request Aug 12, 2023
9 tasks
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.

3 participants