Skip to content

fix: value extraction of point cloud colors enum in display #995

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
JohannesMessner merged 7 commits into from
Jan 9, 2023
Merged

fix: value extraction of point cloud colors enum in display #995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a696c97
docs: fix point cloud display with colors using enum
Jan 6, 2023
3102f7b
fix: revert previous commit
Jan 6, 2023
7925c82
fix: enum value extraction in display point cloud
Jan 6, 2023
18197cb
test: add test for mesh and point cloud enum value extraction
Jan 6, 2023
2a9c256
test: fix test for point cloud enum
Jan 6, 2023
f78c4ad
test: remove useless test again
Jan 6, 2023
0f235d4
fix: value extraction for mesh enum
Jan 6, 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
8 changes: 4 additions & 4 deletions docarray/document/mixins/mesh.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ def load_uri_to_vertices_and_faces(self: 'T') -> 'T':
faces = mesh.faces.view(np.ndarray)

self.chunks = [
Document(name=MeshEnum.VERTICES, tensor=vertices),
Document(name=MeshEnum.FACES, tensor=faces),
Document(name=MeshEnum.VERTICES.value, tensor=vertices),
Document(name=MeshEnum.FACES.value, tensor=faces),
]

return self
Expand All @@ -101,9 +101,9 @@ def load_vertices_and_faces_to_point_cloud(self: 'T', samples: int) -> 'T':
faces = None

for chunk in self.chunks:
if chunk.tags['name'] == MeshEnum.VERTICES:
if chunk.tags['name'] == MeshEnum.VERTICES.value:
Copy link
Copy Markdown
Member

@samsja samsja Jan 9, 2023

Choose a reason for hiding this comment

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

Suggested change
if chunk.tags['name'] == MeshEnum.VERTICES.value:
if chunk.tags['name'] in [MeshEnum.VERTICES.value, MeshEnum.VERTICES]

for sake of compatibility with previous version

Copy link
Copy Markdown
Member

@samsja samsja Jan 9, 2023

Choose a reason for hiding this comment

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

okay it was not released so not needed actually

Copy link
Copy Markdown
Contributor Author

@anna-charlotte anna-charlotte Jan 9, 2023

Choose a reason for hiding this comment

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

yes, I changed the Mesh class to MeshEnum(Enum) in a previous PR, but didn't adjust the enum value extraction accordingly. This PR hasn't been in a release yet, therefore no need to add the MeshEnum.VERTICES.

vertices = chunk.tensor
if chunk.tags['name'] == MeshEnum.FACES:
if chunk.tags['name'] == MeshEnum.FACES.value:
Copy link
Copy Markdown
Member

@samsja samsja Jan 9, 2023

Choose a reason for hiding this comment

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

Suggested change
if chunk.tags['name'] == MeshEnum.FACES.value:
if chunk.tags['name'] in [MeshEnum.VERTICES.value, MeshEnum.VERTICES]

same here

faces = chunk.tensor

if vertices is not None and faces is not None:
Expand Down
17 changes: 11 additions & 6 deletions docarray/document/mixins/plot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,10 @@ def _is_3d_vertices_and_faces(self):
"""
if self.chunks is not None:
name_tags = [c.tags['name'] for c in self.chunks]
if MeshEnum.VERTICES in name_tags and MeshEnum.FACES in name_tags:
if (
MeshEnum.VERTICES.value in name_tags
and MeshEnum.FACES.value in name_tags
):
Comment on lines +136 to +139
Copy link
Copy Markdown
Member

@samsja samsja Jan 9, 2023

Choose a reason for hiding this comment

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

same lets support the old way as well

return True
else:
return False
Expand Down Expand Up @@ -173,11 +176,13 @@ def display_vertices_and_faces(self):
import trimesh

vertices = [
c.tensor for c in self.chunks if c.tags['name'] == MeshEnum.VERTICES
c.tensor
for c in self.chunks
if c.tags['name'] == MeshEnum.VERTICES.value
][-1]
faces = [
c.tensor for c in self.chunks if c.tags['name'] == MeshEnum.FACES.value
][-1]
faces = [c.tensor for c in self.chunks if c.tags['name'] == MeshEnum.FACES][
-1
]
mesh = trimesh.Trimesh(vertices=vertices, faces=faces)
display(mesh.show())

Expand All @@ -191,7 +196,7 @@ def display_point_cloud_tensor(self) -> None:
for chunk in self.chunks:
if (
'name' in chunk.tags.keys()
and chunk.tags['name'] == PointCloudEnum.COLORS
and chunk.tags['name'] == PointCloudEnum.COLORS.value
and chunk.tensor.shape[-1] in [3, 4]
):
colors = chunk.tensor
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/document/test_converters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from docarray import Document
from docarray.document.generators import from_files
from docarray.document.mixins.mesh import MeshEnum
from docarray.document.mixins.mesh import MeshEnum, PointCloudEnum

__windows__ = sys.platform == 'win32'

Expand Down Expand Up @@ -321,9 +321,9 @@ def test_load_uri_to_vertices_and_faces(uri):
doc.load_uri_to_vertices_and_faces()

assert len(doc.chunks) == 2
assert doc.chunks[0].tags['name'] == MeshEnum.VERTICES
assert doc.chunks[0].tags['name'] == MeshEnum.VERTICES.value
assert doc.chunks[0].tensor.shape[1] == 3
assert doc.chunks[1].tags['name'] == MeshEnum.FACES
assert doc.chunks[1].tags['name'] == MeshEnum.FACES.value
assert doc.chunks[1].tensor.shape[1] == 3


Expand Down