Sourcery #10
Merged
Sourcery #10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit introduces a major refactoring of the core engine and adds a new mesh fitting capability. It also includes significant improvements to the development tooling and examples. Core Engine Refactoring: - Introduced `MeshConfig` and `EngineMesh` data structures to provide a more robust and explicit way of handling mesh configurations and transformations. - Refactored the simulation pipeline in `run.py` and `engine.py` to be more modular and easier to follow. - Improved path resolution and handling of settings from both CLI and YAML files. Mesh Fitting Feature: - Added a new `fitting` module to find the best-matching mesh from an HDF5 database based on a target transformation (translation, rotation). - Implemented Chamfer distance calculation to robustly compare mesh similarities. - Added a new example script (`fitting_example.py`) to demonstrate this functionality. Tooling and Examples: - Added new example scripts and corresponding YAML settings files to demonstrate core features like symmetry, rotations, and drafts. - Enhanced the `Justfile` with new commands for development (`install-dev`), running tests with coverage (`test-cov`), and managing examples (`generate-all`, `fleetmaster-all`, `clean-examples`). Fixes: - Resolved several `mypy` type errors in the test suite and CLI command files. - Fixed a `setuptools-scm` warning by removing a redundant version configuration in `pyproject.toml`. - Updated dependencies in `pyproject.toml` and `uv.lock` to resolve conflicts and ensure a stable build. - Updated `CONTRIBUTING.md` with instructions for squashing commits.feat: Refactor core engine and add mesh fitting feature This commit introduces a major refactoring of the core engine and adds a new mesh fitting capability. It also includes significant improvements to the development tooling and examples. Core Engine Refactoring: - Introduced `MeshConfig` and `EngineMesh` data structures to provide a more robust and explicit way of handling mesh configurations and transformations. - Refactored the simulation pipeline in `run.py` and `engine.py` to be more modular and easier to follow. - Improved path resolution and handling of settings from both CLI and YAML files. Mesh Fitting Feature: - Added a new `fitting` module to find the best-matching mesh from an HDF5 database based on a target transformation (translation, rotation). - Implemented Chamfer distance calculation to robustly compare mesh similarities. - Added a new example script (`fitting_example.py`) to demonstrate this functionality. Tooling and Examples: - Added new example scripts and corresponding YAML settings files to demonstrate core features like symmetry, rotations, and drafts. - Enhanced the `Justfile` with new commands for development (`install-dev`), running tests with coverage (`test-cov`), and managing examples (`generate-all`, `fleetmaster-all`, `clean-examples`). Fixes: - Resolved several `mypy` type errors in the test suite and CLI command files. - Fixed a `setuptools-scm` warning by removing a redundant version configuration in `pyproject.toml`. - Updated dependencies in `pyproject.toml` and `uv.lock` to resolve conflicts and ensure a stable build. - Updated `CONTRIBUTING.md` with instructions for squashing commits.
Contributor
Reviewer's GuideThis PR refactors mesh handling and simulation pipelines by introducing structured mesh metadata (MeshConfig, EngineMesh), centralizing transformation and HDF5 operations in dedicated helpers, simplifying categorical dtype checks, and modularizing case processing. The settings model and CLI are updated to use MeshConfig, support base mesh origins, and resolve paths. Sequence diagram for mesh processing and simulation pipelinesequenceDiagram
participant S as "SimulationSettings"
participant E as "EngineMesh"
participant DB as "HDF5 Database"
participant IO as "load_meshes_from_hdf5()"
participant PIPE as "_run_pipeline_for_mesh()"
S->E: Create EngineMesh for each MeshConfig
E->DB: Check for mesh in HDF5 (via IO)
alt Mesh exists and not overwrite
DB->E: Return mesh from database
else Mesh not in DB or overwrite
E->IO: Load/generate mesh from STL
E->DB: Save mesh to HDF5
end
E->PIPE: Run simulation pipeline for mesh
PIPE->DB: Save results to HDF5
ER diagram for mesh and simulation data in HDF5 databaseerDiagram
MESHES {
sha256 string
translation list[float]
rotation list[float]
cog list[float]
inertia_tensor array
stl_content binary
}
CASES {
stl_mesh_name string
transformation_matrix array
cog_for_calculation list[float]
simulation_results array
}
MESHES ||--o{ CASES : contains
MESHES {
volume float
cog_x float
cog_y float
cog_z float
bbox_lx float
bbox_ly float
bbox_lz float
}
Class diagram for new mesh configuration and engine typesclassDiagram
class MeshConfig {
+file: str
+name: str | None
+translation: list[float]
+rotation: list[float]
+cog: list[float] | None
+wave_periods: float | list[float] | None
+wave_directions: float | list[float] | None
+check_vector_length(v)
+check_cog_length(v)
}
class EngineMesh {
+name: str
+mesh: trimesh.Trimesh
+config: MeshConfig
}
class SimulationSettings {
+base_mesh: str | None
+base_origin: list[float] | None
+stl_files: list[MeshConfig]
+output_directory: str | None
+output_hdf5_file: str
+wave_periods: float | list[float]
+wave_directions: float | list[float]
+forward_speed: float | list[float]
+lid: bool
+add_center_of_mass: bool
+grid_symmetry: bool
+update_cases: bool
+combine_cases: bool
+normalize_stl_files(v)
+speed_must_be_non_negative(v)
}
MeshConfig <.. EngineMesh : config
MeshConfig <.. SimulationSettings : used in stl_files
Class diagram for new and updated exceptionsclassDiagram
class SimulationConfigurationError {
}
class InvalidVectorLength {
+__init__(message)
}
class HDF5AttributeError {
+__init__(attribute_name)
}
class MeshLoadError {
+__init__(mesh_name)
}
class DatabaseFileNotFoundError {
+__init__(path)
}
SimulationConfigurationError <|-- InvalidVectorLength
ValueError <|-- HDF5AttributeError
RuntimeError <|-- MeshLoadError
FileNotFoundError <|-- DatabaseFileNotFoundError
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Engine.py has grown very large and touches multiple responsibilities (mesh I/O, HDF5 database ops, pipeline orchestration); consider splitting it into smaller modules (e.g. mesh transformations, database helpers, pipeline runner) to improve readability and maintainability.
- MeshConfig.name is optional and derived later from the file path in multiple places; adding a validator to automatically populate
namefromfile.stemwhen missing would eliminate repetitive derivation logic throughout the pipeline. - The fitting tests depend on a pre-built HDF5 file in
examples/boxship.hdf5, which makes CI runs brittle; consider marking these as integration tests or providing a lightweight fixture to generate a minimal HDF5 in-memory for unit testing.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Engine.py has grown very large and touches multiple responsibilities (mesh I/O, HDF5 database ops, pipeline orchestration); consider splitting it into smaller modules (e.g. mesh transformations, database helpers, pipeline runner) to improve readability and maintainability.
- MeshConfig.name is optional and derived later from the file path in multiple places; adding a validator to automatically populate `name` from `file.stem` when missing would eliminate repetitive derivation logic throughout the pipeline.
- The fitting tests depend on a pre-built HDF5 file in `examples/boxship.hdf5`, which makes CI runs brittle; consider marking these as integration tests or providing a lightweight fixture to generate a minimal HDF5 in-memory for unit testing.
## Individual Comments
### Comment 1
<location> `src/fleetmaster/core/engine.py:128` </location>
<code_context>
- translation_y: float = 0.0,
- translation_z: float = 0.0,
-) -> trimesh.Trimesh:
+def _prepare_trimesh_geometry(stl_file: str, mesh_config: MeshConfig | None = None) -> trimesh.Trimesh:
"""
- Loads an STL file and applies specified translations.
</code_context>
<issue_to_address>
**suggestion:** Consider handling non-existent STL file paths more robustly.
Catching the exception and raising a clearer error will improve usability and debugging, especially since this function is central to mesh operations.
Suggested implementation:
```python
def _prepare_trimesh_geometry(stl_file: str, mesh_config: MeshConfig | None = None) -> trimesh.Trimesh:
"""
Loads an STL file and applies the specified translation and rotation.
The rotation (roll, pitch, yaw) is performed around the center of gravity (cog)
if specified in the mesh_config. If no cog is specified, the mesh's geometric
center of mass is used as the rotation point. If no configuration is given,
the untransformed loaded mesh is returned.
Returns:
A trimesh.Trimesh object representing the transformed geometry.
Raises:
ValueError: If the STL file does not exist or cannot be loaded.
"""
try:
mesh = trimesh.load(stl_file)
except (FileNotFoundError, OSError) as e:
raise ValueError(f"STL file '{stl_file}' does not exist or cannot be loaded: {e}")
except Exception as e:
raise ValueError(f"Failed to load STL file '{stl_file}': {e}")
```
You may need to update the rest of the function to use the loaded `mesh` variable as before. If the mesh loading logic is elsewhere, move it inside this try-except block.
</issue_to_address>
### Comment 2
<location> `src/fleetmaster/core/engine.py:263-267` </location>
<code_context>
+ boat.keep_immersed_part(free_surface=water_level)
+
+ # Check for empty mesh after keep_immersed_part
+ if boat.mesh.vertices.size == 0 or boat.mesh.faces.size == 0:
+ logger.warning("Resulting mesh is empty after keep_immersed_part. Check if water_level is above the mesh.")
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Warn about empty mesh after keep_immersed_part, but consider raising an error.
Downstream code may still operate on an empty mesh. To avoid invalid processing, consider raising an exception or returning None instead of just logging a warning.
```suggestion
boat.keep_immersed_part(free_surface=water_level)
# Check for empty mesh after keep_immersed_part
if boat.mesh.vertices.size == 0 or boat.mesh.faces.size == 0:
logger.error("Resulting mesh is empty after keep_immersed_part. Check if water_level is above the mesh.")
raise ValueError("Boat mesh is empty after keep_immersed_part. Aborting further processing.")
```
</issue_to_address>
### Comment 3
<location> `src/fleetmaster/core/engine.py:587-596` </location>
<code_context>
+def _process_and_save_single_case(
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider handling exceptions during make_database and file writing.
Wrap make_database and database.to_netcdf in try/except blocks to ensure errors are logged and resources are cleaned up, preventing inconsistent file states.
Suggested implementation:
```python
import logging
def _process_and_save_single_case(
boat: Any, # cpt.FloatingBody is not fully typed, use Any to satisfy mypy
mesh_name: str,
case_params: dict[str, Any],
output_file: Path,
origin_translation: npt.NDArray[np.float64] | None,
) -> Any:
"""Process a single simulation case and save its results to the HDF5 file."""
logger = logging.getLogger(__name__)
group_name = _generate_case_group_name(
mesh_name, case_params["water_depth"], case_params["water_level"], case_params["forward_speed"]
)
```
```python
group_name = _generate_case_group_name(
mesh_name, case_params["water_depth"], case_params["water_level"], case_params["forward_speed"]
)
try:
database = make_database(
boat=boat,
mesh_name=mesh_name,
case_params=case_params,
origin_translation=origin_translation,
)
except Exception as e:
logger.error(f"Error during make_database for case {group_name}: {e}", exc_info=True)
return None
try:
database.to_netcdf(output_file)
except Exception as e:
logger.error(f"Error writing NetCDF file for case {group_name} to {output_file}: {e}", exc_info=True)
# Attempt to clean up any partially written file
if output_file.exists():
try:
output_file.unlink()
logger.info(f"Removed incomplete output file: {output_file}")
except Exception as cleanup_error:
logger.error(f"Failed to remove incomplete output file {output_file}: {cleanup_error}", exc_info=True)
return None
return database
```
</issue_to_address>
### Comment 4
<location> `src/fleetmaster/core/settings.py:39-42` </location>
<code_context>
+ default=None, description="Mesh-specific wave directions in degrees. Overrides global settings."
+ )
+
+ @field_validator("translation", "rotation")
+ def check_vector_length(cls, v: list[float]) -> list[float]:
+ if len(v) != 3:
+ msg = "Translation and rotation must a of length 3"
+ raise InvalidVectorLength(msg)
+ return v
+
+ @field_validator("cog")
</code_context>
<issue_to_address>
**suggestion:** Raise a more informative error for invalid vector length.
Consider including the provided value and its length in the error message to make debugging easier.
```suggestion
if len(v) != 3:
msg = (
f"Translation and rotation must be a list of length 3, "
f"but got value {v} with length {len(v)}"
)
raise InvalidVectorLength(msg)
return v
```
</issue_to_address>
### Comment 5
<location> `src/fleetmaster/core/settings.py:86-90` </location>
<code_context>
default=False, description="Combine all calculated cases for a single STL into one multi-dimensional dataset."
)
+ @field_validator("stl_files", mode="before")
+ def normalize_stl_files(cls, v: Any) -> Any:
+ if not isinstance(v, list):
+ return v
+ return [MeshConfig(file=item) if isinstance(item, str) else item for item in v]
+
# field validator checks the value of one specific field inmediately
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider validating that all items in stl_files are unique by file name.
Duplicate file names may cause issues during mesh processing and when storing data in the database.
```suggestion
@field_validator("stl_files", mode="before")
def normalize_stl_files(cls, v: Any) -> Any:
if not isinstance(v, list):
return v
normalized = [MeshConfig(file=item) if isinstance(item, str) else item for item in v]
file_names = [mesh.file if hasattr(mesh, "file") else None for mesh in normalized]
duplicates = {name for name in file_names if file_names.count(name) > 1 and name is not None}
if duplicates:
raise ValueError(f"Duplicate STL file names found: {', '.join(duplicates)}")
return normalized
```
</issue_to_address>
### Comment 6
<location> `src/fleetmaster/core/io.py:29-35` </location>
<code_context>
continue
raw = group["stl_content"][()]
try:
mesh = trimesh.load_mesh(io.BytesIO(raw.tobytes()), file_type="stl")
if isinstance(mesh, trimesh.Trimesh):
+ mesh.metadata["name"] = name # Store the name for later identification
+ # Load attributes if they exist
+ mesh.metadata["translation"] = group.attrs.get("translation")
+ mesh.metadata["rotation"] = group.attrs.get("rotation")
+ mesh.metadata["cog"] = group.attrs.get("cog")
meshes.append(mesh)
except Exception:
</code_context>
<issue_to_address>
**suggestion:** Consider validating loaded mesh before appending.
Skip appending meshes that are not valid trimesh.Trimesh instances or are empty to avoid issues downstream.
```suggestion
if isinstance(mesh, trimesh.Trimesh) and mesh.is_volume and mesh.vertices.shape[0] > 0 and mesh.faces.shape[0] > 0:
mesh.metadata["name"] = name # Store the name for later identification
# Load attributes if they exist
mesh.metadata["translation"] = group.attrs.get("translation")
mesh.metadata["rotation"] = group.attrs.get("rotation")
mesh.metadata["cog"] = group.attrs.get("cog")
meshes.append(mesh)
else:
logger.debug("Mesh %r is not a valid or non-empty Trimesh. Skipping.", name)
```
</issue_to_address>
### Comment 7
<location> `src/fleetmaster/commands/run.py:178` </location>
<code_context>
+ config[key] = parsed_range
+
+
+def _resolve_paths_in_config(config: dict[str, Any], settings_dir: Path) -> None:
+ """Resolves relative paths for 'base_mesh' and 'stl_files' in the config."""
+ # Resolve base_mesh path
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring repeated path resolution logic into a single helper function for clarity and maintainability.
Here’s a small refactoring that centralizes the “resolve one file‐entry” logic and then simply maps over your list. This removes all the repeated `isinstance`/branching in `_resolve_paths_in_config` and keeps the exact same behavior:
```python
from pathlib import Path
from typing import Any, Union
from dataclasses import replace
FileEntry = Union[str, dict[str, Any], MeshConfig]
def _resolve_entry(item: FileEntry, settings_dir: Path) -> FileEntry:
# pull out the raw path
if isinstance(item, str):
raw = item
elif isinstance(item, dict):
raw = item["file"]
else: # MeshConfig
raw = item.file
# make absolute
resolved = str((settings_dir / raw).resolve()) if not Path(raw).is_absolute() else raw
# re-package into same type
if isinstance(item, str):
return resolved
if isinstance(item, dict):
return {**item, "file": resolved}
# MeshConfig is a frozen dataclass? use replace
return replace(item, file=resolved)
def _resolve_paths_in_config(config: dict[str, Any], settings_dir: Path) -> None:
if base := config.get("base_mesh"):
# resolve just like above
config["base_mesh"] = str((settings_dir / base).resolve()) \
if not Path(base).is_absolute() else base
files = config.get("stl_files")
if not files:
return
config["stl_files"] = [
_resolve_entry(item, settings_dir)
for item in files
]
```
• You now have one focused helper (`_resolve_entry`) instead of three separate branches.
• The outer function becomes a single list‐comprehension.
• All functionality (strings, dicts, MeshConfig) remains exactly the same.
</issue_to_address>
### Comment 8
<location> `src/fleetmaster/core/engine.py:689` </location>
<code_context>
def process_all_cases_for_one_stl(
engine_mesh: EngineMesh,
wave_frequencies: list | npt.NDArray[np.float64],
wave_directions: list | npt.NDArray[np.float64],
water_depths: list | npt.NDArray[np.float64],
water_levels: list | npt.NDArray[np.float64],
forwards_speeds: list | npt.NDArray[np.float64],
lid: bool,
grid_symmetry: bool,
output_file: Path,
update_cases: bool = False,
combine_cases: bool = False,
origin_translation: npt.NDArray[np.float64] | None = None,
) -> None:
# 1. Use the prepared (and possibly translated) geometry to create the Capytaine body
boat, final_mesh = _prepare_capytaine_body(
engine_mesh=engine_mesh,
lid=lid,
grid_symmetry=grid_symmetry,
)
# 2. Add the final, immersed mesh geometry to the database. This version is now the translated one.
if final_mesh is not None:
add_mesh_to_database(
output_file, final_mesh, engine_mesh.name, overwrite=update_cases, mesh_config=engine_mesh.config
)
all_datasets = []
for water_level, water_depth, forward_speed in product(water_levels, water_depths, forwards_speeds):
case_params = {
"omegas": wave_frequencies,
"wave_directions": wave_directions,
"water_level": water_level,
"water_depth": water_depth,
"forward_speed": forward_speed,
"update_cases": update_cases,
"combine_cases": combine_cases,
}
result_db = _process_and_save_single_case(boat, engine_mesh.name, case_params, output_file, origin_translation)
if combine_cases and result_db is not None:
all_datasets.append(result_db)
if combine_cases:
if all_datasets:
logger.info("Combining all calculated cases into a single multi-dimensional dataset.")
combined_dataset = xr.combine_by_coords(all_datasets, combine_attrs="drop_conflicts")
combined_group_name = f"{engine_mesh.name}_multi_dim"
logger.info(f"Writing combined dataset to group '{combined_group_name}' in HDF5 file: {output_file}")
with h5py.File(output_file, "a") as f:
if combined_group_name in f:
del f[combined_group_name]
combined_dataset.to_netcdf(output_file, mode="a", group=combined_group_name, engine="h5netcdf")
with h5py.File(output_file, "a") as f:
f[combined_group_name].attrs["stl_mesh_name"] = engine_mesh.name
else:
logger.warning(
"The 'combine_cases' option is enabled, but no datasets were generated to combine. "
"This can happen if all cases were already present in the output file and 'update_cases' was false."
)
logger.debug(f"Successfully wrote all data for mesh '{engine_mesh.name}' to HDF5.")
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Swap positions of nested conditionals ([`swap-nested-ifs`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/swap-nested-ifs/))
- Hoist nested repeated code outside conditional statements ([`hoist-similar-statement-from-if`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/hoist-similar-statement-from-if/))
- Extract code out into function ([`extract-method`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/extract-method/))
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| translation_y: float = 0.0, | ||
| translation_z: float = 0.0, | ||
| ) -> trimesh.Trimesh: | ||
| def _prepare_trimesh_geometry(stl_file: str, mesh_config: MeshConfig | None = None) -> trimesh.Trimesh: |
Contributor
There was a problem hiding this comment.
suggestion: Consider handling non-existent STL file paths more robustly.
Catching the exception and raising a clearer error will improve usability and debugging, especially since this function is central to mesh operations.
Suggested implementation:
def _prepare_trimesh_geometry(stl_file: str, mesh_config: MeshConfig | None = None) -> trimesh.Trimesh:
"""
Loads an STL file and applies the specified translation and rotation.
The rotation (roll, pitch, yaw) is performed around the center of gravity (cog)
if specified in the mesh_config. If no cog is specified, the mesh's geometric
center of mass is used as the rotation point. If no configuration is given,
the untransformed loaded mesh is returned.
Returns:
A trimesh.Trimesh object representing the transformed geometry.
Raises:
ValueError: If the STL file does not exist or cannot be loaded.
"""
try:
mesh = trimesh.load(stl_file)
except (FileNotFoundError, OSError) as e:
raise ValueError(f"STL file '{stl_file}' does not exist or cannot be loaded: {e}")
except Exception as e:
raise ValueError(f"Failed to load STL file '{stl_file}': {e}")You may need to update the rest of the function to use the loaded mesh variable as before. If the mesh loading logic is elsewhere, move it inside this try-except block.
Comment on lines
263
to
267
| boat.keep_immersed_part(free_surface=water_level) | ||
|
|
||
| # Check for empty mesh after keep_immersed_part | ||
| if boat.mesh.vertices.size == 0 or boat.mesh.faces.size == 0: | ||
| logger.warning("Resulting mesh is empty after keep_immersed_part. Check if water_level is above the mesh.") |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Warn about empty mesh after keep_immersed_part, but consider raising an error.
Downstream code may still operate on an empty mesh. To avoid invalid processing, consider raising an exception or returning None instead of just logging a warning.
Suggested change
| boat.keep_immersed_part(free_surface=water_level) | |
| # Check for empty mesh after keep_immersed_part | |
| if boat.mesh.vertices.size == 0 or boat.mesh.faces.size == 0: | |
| logger.warning("Resulting mesh is empty after keep_immersed_part. Check if water_level is above the mesh.") | |
| boat.keep_immersed_part(free_surface=water_level) | |
| # Check for empty mesh after keep_immersed_part | |
| if boat.mesh.vertices.size == 0 or boat.mesh.faces.size == 0: | |
| logger.error("Resulting mesh is empty after keep_immersed_part. Check if water_level is above the mesh.") | |
| raise ValueError("Boat mesh is empty after keep_immersed_part. Aborting further processing.") |
Comment on lines
+39
to
+42
| if len(v) != 3: | ||
| msg = "Translation and rotation must a of length 3" | ||
| raise InvalidVectorLength(msg) | ||
| return v |
Contributor
There was a problem hiding this comment.
suggestion: Raise a more informative error for invalid vector length.
Consider including the provided value and its length in the error message to make debugging easier.
Suggested change
| if len(v) != 3: | |
| msg = "Translation and rotation must a of length 3" | |
| raise InvalidVectorLength(msg) | |
| return v | |
| if len(v) != 3: | |
| msg = ( | |
| f"Translation and rotation must be a list of length 3, " | |
| f"but got value {v} with length {len(v)}" | |
| ) | |
| raise InvalidVectorLength(msg) | |
| return v |
Comment on lines
29
to
35
| if isinstance(mesh, trimesh.Trimesh): | ||
| mesh.metadata["name"] = name # Store the name for later identification | ||
| # Load attributes if they exist | ||
| mesh.metadata["translation"] = group.attrs.get("translation") | ||
| mesh.metadata["rotation"] = group.attrs.get("rotation") | ||
| mesh.metadata["cog"] = group.attrs.get("cog") | ||
| meshes.append(mesh) |
Contributor
There was a problem hiding this comment.
suggestion: Consider validating loaded mesh before appending.
Skip appending meshes that are not valid trimesh.Trimesh instances or are empty to avoid issues downstream.
Suggested change
| if isinstance(mesh, trimesh.Trimesh): | |
| mesh.metadata["name"] = name # Store the name for later identification | |
| # Load attributes if they exist | |
| mesh.metadata["translation"] = group.attrs.get("translation") | |
| mesh.metadata["rotation"] = group.attrs.get("rotation") | |
| mesh.metadata["cog"] = group.attrs.get("cog") | |
| meshes.append(mesh) | |
| if isinstance(mesh, trimesh.Trimesh) and mesh.is_volume and mesh.vertices.shape[0] > 0 and mesh.faces.shape[0] > 0: | |
| mesh.metadata["name"] = name # Store the name for later identification | |
| # Load attributes if they exist | |
| mesh.metadata["translation"] = group.attrs.get("translation") | |
| mesh.metadata["rotation"] = group.attrs.get("rotation") | |
| mesh.metadata["cog"] = group.attrs.get("cog") | |
| meshes.append(mesh) | |
| else: | |
| logger.debug("Mesh %r is not a valid or non-empty Trimesh. Skipping.", name) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Applied all sourcery recommendations to the linear code
Summary by Sourcery
Refactor and enhance mesh management and simulation workflow by introducing structured MeshConfig and EngineMesh objects, extracting transformation logic, improving HDF5 mesh database operations, extending CLI settings handling, and adding a mesh fitting feature with Chamfer distance. Update tests, docs, examples, and CI to align with these changes.
New Features:
Enhancements:
CI:
Documentation:
Tests: