-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
feat: support hff mesh ingestion #234
Conversation
# Conflicts: # ingestion_tools/poetry.lock # ingestion_tools/pyproject.toml # ingestion_tools/scripts/importers/annotation.py # ingestion_tools/scripts/tests/s3_import/test_annotations.py
- include tests cases - support ingesting glb format
# Conflicts: # ingestion_tools/dataset_configs/template.yaml # schema/ingestion_config/v1.0.0/codegen/ingestion_config_models.schema.json
…st-mesh-hff # Conflicts: # ingestion_tools/scripts/importers/annotation.py
# Conflicts: # ingestion_tools/dataset_configs/template.yaml # ingestion_tools/poetry.lock # ingestion_tools/pyproject.toml # ingestion_tools/scripts/common/mesh_converter.py # ingestion_tools/scripts/importers/annotation.py # ingestion_tools/scripts/tests/s3_import/test_annotations.py # schema/api/v1.0.0/codegen/api_models_materialized.yaml # schema/core/v1.1.0/codegen/metadata_materialized.yaml # schema/core/v1.1.0/codegen/metadata_models.py # schema/core/v1.1.0/common.yaml # schema/core/v1.1.0/metadata.yaml # schema/ingestion_config/v1.0.0/codegen/ingestion_config_models.py # schema/ingestion_config/v1.0.0/codegen/ingestion_config_models.schema.json # schema/ingestion_config/v1.0.0/codegen/ingestion_config_models_materialized.yaml # schema/ingestion_config/v1.0.0/ingestion_config_models.yaml
schema/core/v1.1.0/metadata.yaml
Outdated
is_a: AnnotationSourceFile | ||
aliases: | ||
- TriangularMeshGroup | ||
description: File and sourcing data for a triangular mesh annotation. Annotation that identifies an object. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this description be adapted for a mesh group?
mesh.apply_scale(scale_factor) | ||
mesh.export(output_file) | ||
|
||
return wrapper | ||
|
||
|
||
def check_mesh_name(input_file: str, name: str) -> str: | ||
with h5py.File(input_file, "r") as fp: | ||
for mesh_list in fp["segment_list/"].keys(): # noqa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sanity check: Could there be a case, where this doesn't have segment_list/
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There shouldn't be but it's a possibility. I'll handle that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this does happen a key error will be thrown which names the missing key name. How does that sounds?
import trimesh | ||
|
||
|
||
def generate_hff_mesh(input_file: str, output_file: str) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
if __name__ == "__main__": | ||
import os | ||
|
||
# get test data location | ||
cd = os.path.dirname(__file__).split("/")[0:-1] | ||
test_data_location = os.path.join("/", *cd, "fixtures/annotations") | ||
|
||
# create the input and output file paths | ||
file_prefix = os.path.join(test_data_location, "triangular_mesh") | ||
input_file = f"{file_prefix}.glb" | ||
output_file = f"{file_prefix}.hff" | ||
|
||
# generate the hff mesh file | ||
generate_hff_mesh(input_file, output_file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we want to retain this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're talking about the script, yes I think we should retain it. It could be helpful in the future for generating new hff files and fordebugging issues with hff conversion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker: I on the same page with having the generate_hff_mesh
method. I was just curious about the code block inside the if __name__ == "__main__"
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll keep it since it's a simple runnable example on how to generate the hff
# Conflicts: # ingestion_tools/poetry.lock # schema/core/v1.1.0/codegen/metadata_materialized.yaml # schema/core/v1.1.0/codegen/metadata_models.py # schema/ingestion_config/v1.0.0/codegen/ingestion_config_models.py # schema/ingestion_config/v1.0.0/codegen/ingestion_config_models_materialized.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@@ -24,6 +24,7 @@ repos: | |||
- apiv2/pyproject.toml | |||
files: '^apiv2/.*' | |||
- repo: https://github.com/mpalmer/action-validator | |||
# requires installing `npm install -g @action-validator/core @action-validator/cli --save-dev` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
Relates to chanzuckerberg/cryoet-data-portal#951
Blocked by #209
Description
Testing