Skip to content

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

Merged
JohannesMessner merged 7 commits intomainfrom
docs-fix-display-point-cloud
Jan 9, 2023
Merged

fix: value extraction of point cloud colors enum in display#995
JohannesMessner merged 7 commits intomainfrom
docs-fix-display-point-cloud

Conversation

@anna-charlotte
Copy link
Copy Markdown
Contributor

@anna-charlotte anna-charlotte commented Jan 6, 2023

Fix code snippet for point cloud display with color:
Add PointCloudEnum.COLORS as name tag instead of 'point_cloud_colors'

  • check and update documentation, if required. See guide

anna-charlotte added 2 commits January 6, 2023 11:43
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 6, 2023

Codecov Report

Base: 85.18% // Head: 85.04% // Decreases project coverage by -0.13% ⚠️

Coverage data is based on head (0f235d4) compared to base (3e7f514).
Patch coverage: 75.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #995      +/-   ##
==========================================
- Coverage   85.18%   85.04%   -0.14%     
==========================================
  Files         155      155              
  Lines        8057     8057              
==========================================
- Hits         6863     6852      -11     
- Misses       1194     1205      +11     
Flag Coverage Δ
docarray 85.04% <75.00%> (-0.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
docarray/document/mixins/plot.py 65.48% <50.00%> (ø)
docarray/document/mixins/mesh.py 98.36% <100.00%> (ø)
docarray/array/mixins/sample.py 45.45% <0.00%> (-54.55%) ⬇️
docarray/math/ndarray.py 89.11% <0.00%> (-2.05%) ⬇️
docarray/array/mixins/content.py 95.38% <0.00%> (-1.54%) ⬇️
docarray/array/mixins/find.py 88.23% <0.00%> (-0.99%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@anna-charlotte anna-charlotte changed the title docs: fix point cloud display with colors using enum fix: value extraction of point cloud colors enum in display Jan 6, 2023
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 6, 2023

📝 Docs are deployed on https://ft-docs-fix-display-point-cloud--jina-docs.netlify.app 🎉

anna-charlotte added 2 commits January 6, 2023 12:54
Signed-off-by: anna-charlotte <[email protected]>
Signed-off-by: anna-charlotte <[email protected]>
@github-actions github-actions bot added size/xs and removed size/s labels Jan 6, 2023
Signed-off-by: anna-charlotte <[email protected]>
@github-actions github-actions bot added size/s and removed size/xs labels Jan 6, 2023
@anna-charlotte anna-charlotte marked this pull request as ready for review January 9, 2023 08:40

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

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

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

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.

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

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

Comment on lines +136 to +139
if (
MeshEnum.VERTICES.value in name_tags
and MeshEnum.FACES.value in name_tags
):
Copy link
Copy Markdown
Member

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

@JohannesMessner JohannesMessner merged commit 18dd57f into main Jan 9, 2023
@JohannesMessner JohannesMessner deleted the docs-fix-display-point-cloud branch January 9, 2023 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants