Skip to content

feat(v2): display mesh and pointcloud #1113

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.

Sign up for GitHub

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

Merged
anna-charlotte merged 22 commits into from
Feb 16, 2023
Merged

feat(v2): display mesh and pointcloud #1113

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
943d636
feat: display mesh and pointcloud
Feb 9, 2023
e9acd12
chore: update poetry
Feb 9, 2023
c91ab6c
fix: mypy
Feb 9, 2023
e979c32
fix: add display from param to mesh and pc display
Feb 9, 2023
21fb8ab
fix: clean up
Feb 9, 2023
75b33d8
fix: mypy
Feb 9, 2023
7890194
fix: move display from url to mesh and pc url classes
Feb 10, 2023
0129c3a
chore: remove pyglet dependency
Feb 10, 2023
322a718
chore: update pyproject toml
Feb 10, 2023
05d8461
refactor: copy is notebook function from hubble sdk
Feb 10, 2023
0244816
fix: introduce vertices and faces doc
Feb 10, 2023
255795c
fix: introduce points and colors class for point cloud
Feb 10, 2023
db42712
fix: mypy and tests
Feb 10, 2023
dca04ce
docs: add display example to docs
Feb 10, 2023
788f834
fix: apply johannes suggestion from review
Feb 10, 2023
57fb1e1
fix: apply samis suggestion
Feb 10, 2023
3a8dc5e
docs: update docstring
Feb 14, 2023
38f771d
fix: only display in notebook
Feb 15, 2023
8c31318
docs: update docstring
Feb 15, 2023
bbef411
chore: get poetry lock file from feat rewrite v2
Feb 15, 2023
b376ddd
docs: update docstrings
Feb 15, 2023
c623f9c
Merge branch 'feat-rewrite-v2' into feat-display-mesh-v2
Feb 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
docs: update docstrings 
Signed-off-by: anna-charlotte <[email protected]>
  • Loading branch information
anna-charlotte committed Feb 15, 2023
commit b376dddffb8e598ff91c4c297205a549fb648b7f
3 changes: 2 additions & 1 deletion docarray/documents/mesh/vertices_and_faces.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ def validate(

def display(self) -> None:
"""
Plot mesh consisting of vertices and faces in notebook.
Plot mesh consisting of vertices and faces.
To use this you need to install trimesh[easy]: `pip install 'trimesh[easy]'`.
"""
import trimesh
from IPython.display import display
Expand Down
4 changes: 2 additions & 2 deletions docarray/documents/point_cloud/points_and_colors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ def validate(

def display(self) -> None:
"""
Plot point cloud consisting of points in 3D space and optionally colors in
notebook.
Plot point cloud consisting of points in 3D space and optionally colors.
To use this you need to install trimesh[easy]: `pip install 'trimesh[easy]'`.
"""
import trimesh
from IPython.display import display
Expand Down
3 changes: 2 additions & 1 deletion docarray/typing/url/url_3d/mesh_url.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ class MyDoc(BaseDocument):

def display(self) -> None:
Copy link
Member

@JohannesMessner JohannesMessner Feb 10, 2023

Choose a reason for hiding this comment

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

url.display feels a bit weird to me, because in this case it doesn't really display the url, it displays the thing the url points to.
And to do that, it has to load from that url under the hood.

So is it necessary to expose this? Why not url.load().display()?

Copy link
Contributor Author

@anna-charlotte anna-charlotte Feb 10, 2023

Choose a reason for hiding this comment

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

yeah i see your point, but its not quite the same, when displaying it from url as it is now, we can display it with color because we just call .show() on the trimesh instance. For url.load().display() we extract the vertices and faces information but as of right now there is no way to extract the color information. Therefore this displays without colors. I think it would be nice to keep the color display if url content includes this information.
What do you think?

Copy link
Member

@samsja samsja Feb 10, 2023

Choose a reason for hiding this comment

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

but this is true that it is weird to display some data (the color) that we cannot load in our tools

Copy link
Contributor Author

@anna-charlotte anna-charlotte Feb 10, 2023

Choose a reason for hiding this comment

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

yes true, can be a bit misleading or confusing, too. do u suggest to remove it then for the url, and do the url.load().display() if someone doesn't want to load it into the tensors? I think this won't change on trimesh side any time soon, to easily extract color information.

Copy link
Member

@samsja samsja Feb 10, 2023

Choose a reason for hiding this comment

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

let me think about it

Copy link
Member

@JohannesMessner JohannesMessner Feb 15, 2023

Choose a reason for hiding this comment

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

so what's the conclusion here? keep it as it is? I see the point of being able to show colors, so I don't have a strong opinion anymore

Copy link
Member

@samsja samsja Feb 16, 2023

Choose a reason for hiding this comment

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

@anna-charlotte what did you decided ?

Copy link
Contributor Author

@anna-charlotte anna-charlotte Feb 16, 2023

Choose a reason for hiding this comment

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

I would keep it like this, for the colors and also to have the display method for all urls

"""
Plot mesh in notebook from url.
Plot mesh from url.
This loads the Trimesh instance of the 3D mesh, and then displays it.
To use this you need to install trimesh[easy]: `pip install 'trimesh[easy]'`.
"""
from IPython.display import display
Copy link
Member

@samsja samsja Feb 15, 2023

Choose a reason for hiding this comment

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

what ahppened if we are not inside IPython ?

Copy link
Contributor Author

@anna-charlotte anna-charlotte Feb 15, 2023

Choose a reason for hiding this comment

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

I think for mesh and point cloud it would still open a pyglet window and display it, but for the other IPython displays it would just print something like '< IPython.display.Audio obj >'.


Expand Down
4 changes: 3 additions & 1 deletion docarray/typing/url/url_3d/point_cloud_url.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ class MyDoc(BaseDocument):

def display(self, samples: int = 10000) -> None:
"""
Plot point cloud in notebook from url.
Plot point cloud from url.
To use this you need to install trimesh[easy]: `pip install 'trimesh[easy]'`.

First, it loads the point cloud into a :class:`PointsAndColors` object, and then
calls display on it. The following is therefore equivalent:

Expand Down