Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
fix: only display in notebook
Signed-off-by: anna-charlotte <[email protected]>
  • Loading branch information
anna-charlotte committed Feb 15, 2023
commit 38f771d5f7e93b7adf31ee411f2c7d08414b2f1a
4 changes: 3 additions & 1 deletion docarray/documents/mesh/mesh_3d.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class Mesh3D(BaseDocument):
from docarray.typing import AnyEmbedding
from typing import Optional


# extend it
class MyMesh3D(Mesh3D):
name: Optional[Text]
Expand All @@ -65,6 +66,7 @@ class MyMesh3D(Mesh3D):
from docarray import BaseDocument
from docarray.documents import Mesh3D, Text


# compose it
class MultiModalDoc(BaseDocument):
mesh: Mesh3D
Expand All @@ -81,7 +83,7 @@ class MultiModalDoc(BaseDocument):
mmdoc.mesh.bytes = mmdoc.mesh.url.load_bytes()


You can display your 3D mesh from either its url, or its tensors:
You can display your 3D mesh in a notebook from either its url, or its tensors:

.. code-block:: python

Expand Down
12 changes: 5 additions & 7 deletions docarray/documents/point_cloud/points_and_colors.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from docarray.base_document import BaseDocument
from docarray.typing import AnyTensor
from docarray.typing.tensor.abstract_tensor import AbstractTensor
from docarray.utils.misc import is_notebook, is_tf_available, is_torch_available
from docarray.utils.misc import is_tf_available, is_torch_available

torch_available = is_torch_available()
if torch_available:
Expand Down Expand Up @@ -46,7 +46,8 @@ def validate(

def display(self) -> None:
"""
Plot point cloud consisting of points in 3D space and optionally colors.
Plot point cloud consisting of points in 3D space and optionally colors in
notebook.
"""
import trimesh
from IPython.display import display
Expand All @@ -61,8 +62,5 @@ def display(self) -> None:
)
pc = trimesh.points.PointCloud(vertices=self.points, colors=colors)

if is_notebook():
s = trimesh.Scene(geometry=pc)
display(s.show())
else:
display(pc.show())
s = trimesh.Scene(geometry=pc)
display(s.show())
2 changes: 1 addition & 1 deletion docarray/typing/url/url_3d/mesh_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class MyDoc(BaseDocument):

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

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

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

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

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

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

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

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

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 from url.
Plot mesh in notebook from url.
This loads the Trimesh instance of the 3D mesh, and then displays it.
"""
from IPython.display import display
Copy link
Member

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

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: 2 additions & 2 deletions docarray/typing/url/url_3d/point_cloud_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class MyDoc(BaseDocument):

def display(self, samples: int = 10000) -> None:
"""
Plot point cloud from url.
Plot point cloud in notebook from url.
First, it loads the point cloud into a :class:`PointsAndColors` object, and then
calls display on it. The following is therefore equivalent:

Expand All @@ -90,7 +90,7 @@ def display(self, samples: int = 10000) -> None:
pc.url.display()

# option 2 (equivalent)
pc.url.load().display()
pc.url.load(samples=10000).display()

:param samples: number of points to sample from the mesh.
"""
Expand Down
26 changes: 0 additions & 26 deletions docarray/utils/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,29 +20,3 @@ def is_torch_available():

def is_tf_available():
return tf_imported


def is_notebook() -> bool:
"""
Check if we're running in a Jupyter notebook, using magic command
`get_ipython` that only available in Jupyter.

:return: True if run in a Jupyter notebook else False.
"""

try:
shell = get_ipython().__class__.__name__ # type: ignore
except NameError:
return False

if shell == 'ZMQInteractiveShell':
return True

elif shell == 'Shell':
return True

elif shell == 'TerminalInteractiveShell':
return False

else:
return False