fix: clean up

Signed-off-by: anna-charlotte <[email protected]>
docarray · anna-charlotte · Feb 16, 2023 · Feb 9, 2023 · Feb 9, 2023 · Feb 9, 2023
commit 21fb8ab3c5ec597bd31fd856032bb14843b0e26c
diff --git a/docarray/documents/mesh.py b/docarray/documents/mesh.py
@@ -94,28 +94,27 @@ def validate(
             value = cls(url=value)
         return super().validate(value)
 
-    def display(self, display_from: str = 'url'):
+    def display(self, display_from: str = 'url') -> None:
         """
         Plot mesh consisting of vertices and faces.
+        :param display_from: display from either url or tensors (vertices and faces).
         """
+        import trimesh
         from IPython.display import display
 
         if display_from not in ['tensor', 'url']:
             raise ValueError(f'Expected one of ["tensor", "url"], got "{display_from}"')
 
-        if not getattr(self, display_from):
-            raise ValueError(
-                f'Can not to display point cloud from {display_from} when the '
-                f'{display_from} is None.'
-            )
-
-        if self.url:
-            # mesh from uri
+        if display_from == 'url':
+            if self.url is None:
+                raise ValueError('Can\'t display mesh from url when the url is None.')
             mesh = self.url._load_trimesh_instance()
             display(mesh.show())
         else:
-            # mesh from vertices and faces tensors
-            import trimesh
+            if self.vertices is None or self.faces is None:
+                raise ValueError(
+                    'Can\'t display mesh from tensor when vertices and/or faces is None'
+                )
 
             mesh = trimesh.Trimesh(vertices=self.vertices, faces=self.faces)
             display(mesh.show())
diff --git a/docarray/documents/point_cloud.py b/docarray/documents/point_cloud.py
@@ -116,6 +116,9 @@ def validate(
     def display(self, display_from: str = 'url', samples: int = 10000) -> None:
         """
         Plot interactive point cloud from :attr:`.tensor`
+        :param display_from: display point cloud from either url or tensor.
+        :param samples: number of points to sample from the mesh, will be ignored if
+            displayed from tensor.
         """
         import trimesh
         from hubble.utils.notebook import is_notebook
@@ -124,7 +127,7 @@ def display(self, display_from: str = 'url', samples: int = 10000) -> None:
         if display_from not in ['tensor', 'url']:
             raise ValueError(f'Expected one of ["tensor", "url"], got "{display_from}"')
 
-        if not getattr(self, display_from):
+        if getattr(self, display_from) is None:
             raise ValueError(
                 f'Can not to display point cloud from {display_from} when the '
                 f'{display_from} is None.'

diff --git a/tests/integrations/predefined_document/test_mesh.py b/tests/integrations/predefined_document/test_mesh.py
@@ -44,6 +44,9 @@ def test_display_illegal_param():
     with pytest.raises(ValueError):
         mesh.display(display_from='tensor')
 
-    mesh = Mesh3D(vertices=np.zeros((10, 3)), faces=np.ones(10, 3))
+    mesh = Mesh3D(vertices=np.zeros((10, 3)), faces=np.ones((10, 3)))
     with pytest.raises(ValueError):
         mesh.display(display_from='url')
+
+    with pytest.raises(ValueError):
+        mesh.display(display_from='illegal')
diff --git a/tests/integrations/predefined_document/test_point_cloud.py b/tests/integrations/predefined_document/test_point_cloud.py
@@ -83,3 +83,6 @@ def test_display_illegal_param():
     pc = PointCloud3D(tensor=np.zeros((10, 3)))
     with pytest.raises(ValueError):
         pc.display(display_from='url')
+
+    with pytest.raises(ValueError):
+        pc.display(display_from='illegal')