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

Avoid import star (from open3d import *) #982

Merged
merged 9 commits into from May 23, 2019
Merged

Conversation

yxlao
Copy link
Collaborator

@yxlao yxlao commented May 23, 2019

  • Replaced from open3d import *
  • All python code formatted with YAPF with .style.yapf
  • Docs have updated line numbers when referring to the python code

This change is Reviewable

Copy link
Contributor

@griegler griegler left a comment

Choose a reason for hiding this comment

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

I like this. This should make the documentation a lot easier to read and to directly use snippets from the examples.

Reviewed 99 of 99 files at r1.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @qianyizh, @syncle, and @yxlao)


.style.yapf, line 2 at r1 (raw file):

[style]
based_on_style = google

Can we have this as a target in cmake, or better combine it with the existing make check-style and make apply-style?


examples/Python/Basic/convex_hull.py, line 5 at r1 (raw file):

# See license file or visit www.open3d.org for details

# examples/Python/Basic/mesh_sampling.py

convex_hull.py


examples/Python/Basic/half_edge_mesh.py, line 12 at r1 (raw file):

def draw_geometries_with_back_face(geometries):
    visualizer = o3d.o3d.visualization.Visualizer()

o3d.o3d ?


examples/Python/Basic/half_edge_mesh.py, line 23 at r1 (raw file):

if __name__ == "__main__":
    mesh = o3d.o3d.io.read_triangle_mesh("../../TestData/sphere.ply")

o3d.o3d ?


examples/Python/Basic/half_edge_mesh.py, line 24 at r1 (raw file):

if __name__ == "__main__":
    mesh = o3d.o3d.io.read_triangle_mesh("../../TestData/sphere.ply")
    mesh = o3d.o3d.geometry.crop_triangle_mesh(mesh, [-1, -1, -1], [1, 0.6, 1])

o3d.o3d ?


examples/Python/Basic/half_edge_mesh.py, line 26 at r1 (raw file):

    mesh = o3d.o3d.geometry.crop_triangle_mesh(mesh, [-1, -1, -1], [1, 0.6, 1])
    mesh.purge()
    mesh = o3d.o3d.geometry.create_half_edge_mesh_from_mesh(mesh)

o3d.o3d ?


examples/Python/Basic/half_edge_mesh.py, line 39 at r1 (raw file):

    vertex_colors = 0.75 * np.ones((num_vertices, 3))
    vertex_colors[boundary_vertices, :] = np.array([1, 0, 0])
    mesh.vertex_colors = o3d.o3d.utility.Vector3dVector(vertex_colors)

o3d.o3d ?


examples/Python/Basic/mesh_simplification.py, line 5 at r1 (raw file):

# See license file or visit www.open3d.org for details

# examples/Python/Basic/mesh_sampling.py

mesh_simplification.py


examples/Python/Basic/mesh_subdivision.py, line 5 at r1 (raw file):

# See license file or visit www.open3d.org for details

# examples/Python/Basic/mesh_sampling.py

mesh_subdivision.py


examples/Python/ReconstructionSystem/debug/pairwise_pc_alignment.py, line 10 at r1 (raw file):

import json
import sys
import open3d as o3d

not needed here?

Copy link
Collaborator Author

@yxlao yxlao 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 quick review @griegler !

Reviewable status: 90 of 104 files reviewed, 10 unresolved discussions (waiting on @griegler, @qianyizh, and @syncle)


.style.yapf, line 2 at r1 (raw file):

Previously, griegler (Gernot) wrote…

Can we have this as a target in cmake, or better combine it with the existing make check-style and make apply-style?

Done.
Good idea


examples/Python/Basic/convex_hull.py, line 5 at r1 (raw file):

Previously, griegler (Gernot) wrote…

convex_hull.py

Done.


examples/Python/Basic/half_edge_mesh.py, line 12 at r1 (raw file):

Previously, griegler (Gernot) wrote…

o3d.o3d ?

Done.


examples/Python/Basic/half_edge_mesh.py, line 23 at r1 (raw file):

Previously, griegler (Gernot) wrote…

o3d.o3d ?

Done.


examples/Python/Basic/half_edge_mesh.py, line 24 at r1 (raw file):

Previously, griegler (Gernot) wrote…

o3d.o3d ?

Done.


examples/Python/Basic/half_edge_mesh.py, line 26 at r1 (raw file):

Previously, griegler (Gernot) wrote…

o3d.o3d ?

Done.


examples/Python/Basic/half_edge_mesh.py, line 39 at r1 (raw file):

Previously, griegler (Gernot) wrote…

o3d.o3d ?

Done.


examples/Python/Basic/mesh_simplification.py, line 5 at r1 (raw file):

Previously, griegler (Gernot) wrote…

mesh_simplification.py

Done.


examples/Python/Basic/mesh_subdivision.py, line 5 at r1 (raw file):

Previously, griegler (Gernot) wrote…

mesh_subdivision.py

Done.


examples/Python/ReconstructionSystem/debug/pairwise_pc_alignment.py, line 10 at r1 (raw file):

Previously, griegler (Gernot) wrote…

not needed here?

Done.

Copy link
Contributor

@griegler griegler left a comment

Choose a reason for hiding this comment

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

LGTM.
Should we also update the docs:
http://www.open3d.org/docs/compilation.html (at least the gtest part is outdated now)
http://www.open3d.org/docs/contribute.html

Reviewable status: 90 of 104 files reviewed, 10 unresolved discussions (waiting on @griegler, @qianyizh, and @syncle)

@yxlao
Copy link
Collaborator Author

yxlao commented May 23, 2019

Thanks @griegler , could you click "approve" the PR?
I'll update the docs on gtest and python together in my upcoming doc update PR.

Copy link
Contributor

@griegler griegler left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 14 of 14 files at r2.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @griegler, @qianyizh, and @syncle)

@yxlao yxlao merged commit d50699b into isl-org:master May 23, 2019
@yxlao yxlao deleted the python-import branch May 23, 2019 19:55
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

2 participants