jf/opzomeren by jf--- · Pull Request #177 · compas-dev/compas_slicer

jf--- · 2025-12-11T16:19:13Z

@ioannaMitropoulou @joburger & @stratocaster dear compas_slicer authors

The last week I’ve been working on compas_slicer.
I’ve refactored the code significantly:

For code quality
To add compas_viewer methods such that visualisation can be handled via compas_viewer
Speed: I’ve added compas_cgal methods which result in a significant ~10x speedup
The docs have been refactored to mkdocs
A yak workflow (rhino package management) is in the works

via this PR I'd love to join you as co-maintainer

Build System: - Migrate from setup.py/setup.cfg to pyproject.toml - Remove MANIFEST.in, pytest.ini (config now in pyproject.toml) - Update Python requirement to >=3.9 - Replace flake8/autopep8/isort with ruff for linting/formatting - Add mypy for optional type checking Dependencies: - compas>=2.0 (was <2.0) - numpy>=1.24 (remove restrictive upper bound) - networkx>=3.0 - progressbar2>=4.0 - pyclipper>=1.3 - libigl>=2.5 COMPAS 2.x Migration: - Replace compas._os.absjoin() with pathlib.Path - Update pairwise import: compas.utilities -> compas.itertools - Replace mesh_weld(mesh) with mesh.weld() method - Modernize compas_slicer_ghpython install.py for COMPAS 2.x Python 3.9+ Modernization: - Remove all __future__ imports (14 files) - Remove explicit (object) inheritance from classes - Use pathlib.Path throughout - Use f-strings consistently CI/CD: - Test matrix: Python 3.9/3.10/3.11/3.12 - Test on ubuntu/windows/macos - Use ruff for linting, mypy for type checking - Update GitHub Actions to v4/v5 Tasks: - Add invoke tasks: lint, format, typecheck - Update release task to use python -m build

- geometry/path.py: full type annotations for Path class - geometry/layer.py: type hints for Layer, VerticalLayer, VerticalLayersManager - geometry/print_point.py: full type annotations for PrintPoint - slicers/base_slicer.py: type hints for BaseSlicer base class Uses Python 3.9+ style hints with `from __future__ import annotations` for forward references and modern union syntax (X | Y).

- convert % formatting to f-strings - fix SIM102/SIM103 (collapsible if) - fix B007 (unused loop vars) - fix B904 (raise from err) - update mesh_bounding_box -> bounding_box(vertices) - resolve circular imports via direct module imports

- mesh.edge_faces() now takes edge tuple instead of u/v kwargs - fix Path import in layer.py (needed at runtime for isinstance) - fix seams_align import in generate_brim.py

Major refactoring to use typed dataclasses instead of unstructured dicts. All dataclasses inherit from compas.data.Data for COMPAS serialization. New dataclasses: - GcodeParameters: typed G-code params with 25+ fields and defaults - PrintPath: wraps list[PrintPoint] with iteration/indexing - PrintLayer: wraps list[PrintPath] with iteration/indexing - PrintPointsCollection: replaces dict[str, dict[str, list[PrintPoint]]] Converted to dataclasses: - PrintPoint: @DataClass(Data) with __data__/__from_data__ - Path: @DataClass(Data) with legacy dict format compat - Layer/VerticalLayer: @DataClass(Data) with inheritance Key changes: - Integer indices instead of string keys ('layer_0' -> [0]) - Access: collection[0][1][2] vs dict['layer_0']['path_1'][2] - Type-safe with IDE autocomplete - printpoints_dict property for backward compat - Updated all print organizers and utilities

…ad/cut

- new config.py: SlicerConfig, InterpolationConfig, GcodeConfig, PrintConfig dataclasses - enums: GeodesicsMethod, UnionMethod for type-safe config - full type hints: planar/interpolation/scalar_field/uv slicers - full type hints: planar/interpolation/scalar_field print organizers - fix direct imports in planar_slicer (create_planar_paths) - fix mypy issues (optional types, null checks)

- type hints: get_param, defaults_layers, defaults_gcode, defaults_interpolation_slicing - add min_layer_height, max_layer_height to layers defaults - add filament_diameter key (underscore) alongside legacy space key - deprecation note on GcodeParameters (prefer GcodeConfig)

- add _numpy_ops.py with vectorized helpers (KDTree, batch distances) - add test_performance.py with benchmarks + regression tests - vectorize gradient.py (vertex/face/edge gradient, divergence) - vectorize topological_sorting.py (point cloud neighbor check) - add KDTree to mesh_attributes_handling.py (restore_mesh_attributes) - pre-factor sparse matrix in geodesics.py heat diffusion (250x speedup) - vectorize data_smoothing.py iterative loop - vectorize sort_paths_minimum_travel_time.py (seam adjustment) - vectorize seams_align.py (distance calculations) - add numba, pytest-benchmark to deps Test fixtures use Sphere (Box ignores u,v params): - small: ~180 faces - medium: ~2k faces - large: ~8k faces Benchmark results: - vertex gradient (8k faces): ~860μs - divergence (2k faces): ~125μs - batch closest points (1k pts): ~650μs

convert list back to Vector in set_ppt_up_vec

curved slicing: 22s -> 13.5s (38% speedup)

curved slicing: 13.5s -> 12s

- generate_brim: CGAL offset_polygon w/ pyclipper fallback - offset_polygon_with_holes for complex slices (CGAL-only) - remesh_mesh via CGAL trimesh_remesh for mesh quality

- add get_cgal_HEAT_geodesic_distances using compas_cgal.HeatGeodesicSolver - add HEAT_CGAL enum to GeodesicsMethod - CGAL heat ~2.1s vs libigl ~3.1s in full workflow (30% faster)

- add pure NumPy cotmatrix/massmatrix/cotans in utils.py - geodesics.py uses CGAL HeatGeodesicSolver - gradient.py removes use_igl param, always vectorized - region_split.py adds pure Python mesh cut/components - update defaults to heat_cgal method - keep backwards-compatible aliases

- new post_processing/infill module - generate_medial_axis_infill() extracts skeleton edges as infill paths - uses interior_straight_skeleton from compas_cgal - add example 7

…or mkdocs

…ization

- 01 planar slicing: step-by-step walkthrough with brim/raft/seam control - 02 curved slicing: complete with math background and ETH references - 03 vertical sorting: explain branching geometry handling - 04 gcode: document GcodeConfig params and volumetric extrusion math - 05 scalar field: custom distance fields and isocurve extraction - 06 attributes: face vs vertex attributes with barycentric interpolation

- replace 27 assert statements with ValueError/TypeError/RuntimeError - convert old-style string concatenation to f-strings throughout - improves reliability with python -O flag

- remove dead 'default' heat diffusion method that had boundary drift bug - simplify diffuse_heat() to only use working backward Euler iteration - change default geodesics_method from heat_igl to heat_cgal - CGAL uses intrinsic Delaunay triangulation for better accuracy - add docstrings explaining custom vs CGAL implementations

- add mike to docs dependencies - update workflow to deploy with mike - only trigger on docs/ changes - master deploys to latest/stable - enable manual workflow dispatch

jf--- · 2025-12-11T16:24:42Z

merging this PR shouldn't happen before the PRs in compas_cgal are merged and pypi builds updated.
also the GH actions build is to be updated.

jf--- · 2025-12-11T16:26:27Z

sorry for inviting many reviewers; consensus seems called for

Copilot

Pull request overview

This is a comprehensive refactoring of the compas_slicer package that modernizes the codebase with significant improvements to code quality, performance, and maintainability. The PR introduces:

Key Changes

Modernization: Added type hints throughout, removed Python 2 compatibility code (from __future__ imports), updated to use pathlib.Path instead of os.path
Performance: Introduced vectorized NumPy operations (~10x speedup), added CGAL integration for faster planar slicing, created comprehensive performance benchmarks
Documentation: Migrated from Sphinx to MkDocs
Tooling: Updated from flake8 to ruff for linting/formatting, added mypy type checking, modernized build system
Architecture: Refactored data structures (introduced PrintPointsCollection), improved gradient computation with pure Python implementations as fallbacks
Testing: Added performance regression tests, example integration tests

Reviewed changes

Copilot reviewed 176 out of 178 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
tests/test_planar_slicing.py	Updated imports, modernized path handling, removed `slicer_type` parameter
tests/test_planar_print_organization_horizontal_layers.py	Updated to use `Path`, removed commented code
tests/test_performance.py	New comprehensive performance benchmarks with regression guards
tests/test_examples.py	New integration tests for examples
temp/PLACEHOLDER	Removed temporary file placeholder
tasks.py	Modernized invoke tasks, added ruff/mypy/format commands, integrated yak workflow
src/compas_slicer_ghpython/yak_template/*	Added Grasshopper package management files
src/compas_slicer_ghpython/visualization.py	Updated to use `Path`, modernized string formatting
src/compas_slicer_ghpython/install.py	Added rhino dependency checks, improved error handling
src/compas_slicer/visualization/*	New visualization module using compas_viewer
src/compas_slicer/utilities/*	Extensive refactoring with type hints, vectorized operations, added pure Python mesh utilities
src/compas_slicer/slicers/*	Removed legacy planar slicing, standardized to CGAL, added type hints and config objects
src/compas_slicer/print_organization/*	Major refactoring of printpoints data structure, improved G-code generation
src/compas_slicer/pre_processing/*	Vectorized computations, added fallback implementations
src/compas_slicer/post_processing/*	Minor type hint additions
pytest.ini	Removed (configuration likely moved to pyproject.toml)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-11T16:28:44Z

+        paths_data = data["paths"]
+        # Handle both list format and legacy dict format
+        if isinstance(paths_data, dict):
+            paths = [Path.from_data(paths_data[key]) for key in sorted(paths_data.keys(), key=lambda x: int(x))]


This 'lambda' is just a simple wrapper around a callable object. Use that object directly.

Copilot · 2025-12-11T16:28:44Z

+        paths_data = data["paths"]
+        # Handle both list format and legacy dict format
+        if isinstance(paths_data, dict):
+            paths = [Path.from_data(paths_data[key]) for key in sorted(paths_data.keys(), key=lambda x: int(x))]


This 'lambda' is just a simple wrapper around a callable object. Use that object directly.

Copilot · 2025-12-11T16:28:45Z

+        if isinstance(points_data, dict):
+            pts = [
+                Point.__from_data__(points_data[key])
+                for key in sorted(points_data.keys(), key=lambda x: int(x))


This 'lambda' is just a simple wrapper around a callable object. Use that object directly.

Copilot · 2025-12-11T16:28:45Z

+    fan_on = False
+    prev_pt = Point(0, 0, 0)
+    prev_z = 0.0
+    layer_height = PURGE_HEIGHT


This assignment to 'layer_height' is unnecessary as it is redefined before this value is used.

Copilot · 2025-12-11T16:28:46Z


-logger = logging.getLogger('logger')
-logging.basicConfig(format='%(levelname)s-%(message)s', level=logging.INFO)
+import compas_slicer.utilities as utils


Module 'compas_slicer.utilities' is imported with both 'import' and 'import from'.

Copilot · 2025-12-11T16:28:47Z

 from compas_slicer.geometry import Layer
-import logging
-logger = logging.getLogger('logger')
+from loguru import logger


Import of 'logger' is not used.

Copilot · 2025-12-11T16:28:47Z

 import copy
 import networkx as nx
-import logging
+from loguru import logger


Import of 'logger' is not used.

Copilot · 2025-12-11T16:28:47Z


-        self.G = nx.DiGraph()
+        self.G: nx.DiGraph = nx.DiGraph()
        self.create_graph_nodes()


This call to DirectedGraph.create_graph_nodes in an initialization method is overridden by MeshDirectedGraph.create_graph_nodes.
This call to DirectedGraph.create_graph_nodes in an initialization method is overridden by SegmentsDirectedGraph.create_graph_nodes.

Copilot · 2025-12-11T16:28:47Z

-        self.G = nx.DiGraph()
+        self.G: nx.DiGraph = nx.DiGraph()
        self.create_graph_nodes()
        self.root_indices = self.find_roots()


This call to DirectedGraph.find_roots in an initialization method is overridden by MeshDirectedGraph.find_roots.
This call to DirectedGraph.find_roots in an initialization method is overridden by SegmentsDirectedGraph.find_roots.

Copilot · 2025-12-11T16:28:48Z

+        logger.info(f'Graph roots: {self.root_indices}')
+        if len(self.root_indices) == 0:
+            raise ValueError("No root nodes were found. At least one root node is needed.")
        self.end_indices = self.find_ends()


This call to DirectedGraph.find_ends in an initialization method is overridden by MeshDirectedGraph.find_ends.
This call to DirectedGraph.find_ends in an initialization method is overridden by SegmentsDirectedGraph.find_ends.

gonzalocasas · 2025-12-11T17:03:16Z

This is amazing!! Thanks a lot @jf--- !!!!
@joburger @ioannaMitropoulou would you have time to review this change?

tomvanmele · 2025-12-11T17:44:40Z

@jf--- really cool indeed.

maybe the review is easier if you split off at least the docs part to a different PR... :)

jf--- · 2025-12-11T20:24:26Z

@gonzalocasas & @tomvanmele what about the following:

tests are passing
code is in dramatically better shape
I'm reasonable sure the orginal authors aren't going to review -- delving into a codebase from years ago isn't such a reasonable request
the pedantry pissing contest that is copilot isn't reporting anything dramatic
I think fatalistically hitting merge is quite reasonable here; the orphan has seen some cuddling and is more of a milky baby, after living out there on the streets for a few rough years 😈🙀
- I guess some level of consensus is what I'm seeking, pouring over this commit-by-commit would be a waste of precious time -- given the long neglect
- serious talent and resources were spend on crafting this, this is I guess looking for a hattip from those in a postion of authority 👮🏻
see at my fork if nothing is looking all too emberrassing
@ioannaMitropoulou thesis is amazing, I managed to factor out matlab 🥶 from her SDQ_meshes repo; finally got to see your amazing sw. it would be much more interesting to discuss a compas_slicer back port ;)

tomvanmele · 2025-12-11T20:28:43Z

sounds good to me

jf--- · 2025-12-11T20:32:58Z

@gonzalocasas would you do the honors pls? if you could double check this does justice to the original authors, I think that's the pressing thing.

jf--- added 30 commits December 9, 2025 16:07

fix COMPAS 2.x API: edge_faces, fix circular imports

88bc60f

- mesh.edge_faces() now takes edge tuple instead of u/v kwargs - fix Path import in layer.py (needed at runtime for isinstance) - fix seams_align import in generate_brim.py

COMPAS 2.x serialization: __data__ and __from_data__ API

c11b34b

fix mypy errors: direct imports, type annotations, mypy config

d368f46

unify deps: environment.yml only, prefer conda

e80aee7

use compas_libigl for geodesics/massmatrix, keep igl for cotmatrix/gr…

d9c48b2

…ad/cut

refactor: replace igl with compas_libigl, drop libigl dep

76c2f6f

refactor: replace os.path with pathlib.Path

fe53a41

type hints utils.py, Path return for get_output_directory

c0cb5af

type hints post_processing, slice_utilities, planar_slicing

22ca36d

type hints pre_processing main modules, update save_to_json sig

6996247

type hints preprocessing_utils, print_organization_utilities

89a081e

add examples to test suite, add compas_cgal dep, fix compas 2.x compat

acfd2c0

fix up_vector serialization after smoothing

448941f

convert list back to Vector in set_ppt_up_vec

unify rdp + add batch closest point, CGAL fallback

34577cd

curved slicing: 22s -> 13.5s (38% speedup)

perf: geodesic caching + vectorized contour finding

28ea19c

curved slicing: 13.5s -> 12s

add CGAL features: polygon offset, holes, remesh

be25b65

- generate_brim: CGAL offset_polygon w/ pyclipper fallback - offset_polygon_with_holes for complex slices (CGAL-only) - remesh_mesh via CGAL trimesh_remesh for mesh quality

vectorize assign_interpolation_distance, 11s->9.7s

9ce4f8f

simplify calculate_z_bounds

bacf624

switch geodesics default to heat_igl: 22s->3.4s (85%)

bcf4a31

add CGAL heat geodesics method option

bb2fbb9

- add get_cgal_HEAT_geodesic_distances using compas_cgal.HeatGeodesicSolver - add HEAT_CGAL enum to GeodesicsMethod - CGAL heat ~2.1s vs libigl ~3.1s in full workflow (30% faster)

add medial axis infill using CGAL straight skeleton

cc3d7ff

- new post_processing/infill module - generate_medial_axis_infill() extracts skeleton edges as infill paths - uses interior_straight_skeleton from compas_cgal - add example 7

align environment.yml with pyproject.toml deps

83b629a

jf--- added 10 commits December 10, 2025 12:34

cleanup: remove legacy files, convert changelog to md, update tasks f…

219009f

…or mkdocs

docs: add concepts section with architecture, algorithms, print organ…

f685958

…ization

replace asserts with proper exceptions, use f-strings

4cb6226

- replace 27 assert statements with ValueError/TypeError/RuntimeError - convert old-style string concatenation to f-strings throughout - improves reliability with python -O flag

add medial axis infill example documentation

aee2067

add mike for versioned docs

80c64ab

- add mike to docs dependencies - update workflow to deploy with mike - only trigger on docs/ changes - master deploys to latest/stable - enable manual workflow dispatch

add mathjax support for math rendering

bd64b9d

fix ascii art rendering: preserve whitespace in code blocks

e3024bb

update the authors, weazle myself in

d004139

jf--- changed the title ~~Jf/code quality~~ jf/opzomeren Dec 11, 2025

jf--- mentioned this pull request Dec 11, 2025

perf: 90% speedup via CGAL integration #174

Closed

2 tasks

jf--- requested review from Copilot and ioannaMitropoulou December 11, 2025 16:24

jf--- assigned joburger Dec 11, 2025

Copilot started reviewing on behalf of jf--- December 11, 2025 16:25 View session

jf--- unassigned joburger Dec 11, 2025

jf--- requested review from joburger, stratocaster and tomvanmele December 11, 2025 16:25

Copilot AI reviewed Dec 11, 2025

View reviewed changes

ioannaMitropoulou approved these changes Dec 12, 2025

View reviewed changes

ioannaMitropoulou merged commit 1029bd0 into compas-dev:master Dec 12, 2025
9 of 25 checks passed

Conversation

jf--- commented Dec 11, 2025

Uh oh!

jf--- commented Dec 11, 2025

Uh oh!

jf--- commented Dec 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

gonzalocasas commented Dec 11, 2025

Uh oh!

tomvanmele commented Dec 11, 2025

Uh oh!

jf--- commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomvanmele commented Dec 11, 2025

Uh oh!

jf--- commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jf--- commented Dec 11, 2025 •

edited

Loading

jf--- commented Dec 11, 2025 •

edited

Loading