Add scale component handling to Shape and PlanarSurface transformations
Description
This PR fixes the issue where scale components in transformations were being ignored when applied to Shape and Surface subclasses. Previously, calling .transform() or .transformed() with a transformation containing a scale component would only transform the shape's frame while leaving dimensional parameters unchanged.
Changes Made
Shape Subclasses
- Box: Applies non-uniform scaling to xsize, ysize, zsize based on X, Y, Z scale factors
- Sphere: Applies average of scale factors to radius to maintain spherical shape
- Cylinder: Applies average of X,Y scale factors to radius, Z scale factor to height
- Cone: Applies average of X,Y scale factors to radius, Z scale factor to height
- Capsule: Applies average of X,Y scale factors to radius, Z scale factor to height
- Torus: Applies average of X,Y scale factors to radius_axis, Z scale factor to radius_pipe
PlanarSurface
- PlanarSurface: Applies X scale factor to xsize, Y scale factor to ysize
Base Class Changes
-
Shape.transform(): Changed from transforming the frame to raising
NotImplementedErrorfor consistency with other transformation methods likescale() - All shape subclasses override
transform()to decompose transformations and extract scale components before applying them to dimensional parameters
Testing
- Added comprehensive test suite in
tests/compas/geometry/test_shapes_transformations.pywith 29 tests covering:- Uniform and non-uniform scaling for all shape types and PlanarSurface
- Both
transform()andtransformed()methods - Combined transformations (translation + rotation + scale)
- Volume verification after transformation
What type of change is this?
- [x] Bug fix in a backwards-compatible manner.
- [ ] New feature in a backwards-compatible manner.
- [ ] Breaking change: bug fix or new feature that involve incompatible API changes.
- [ ] Other (e.g. doc update, configuration, etc)
Checklist
- [ ] I added a line to the
CHANGELOG.mdfile in theUnreleasedsection under the most fitting heading (e.g.Added,Changed,Removed). - [x] I ran all tests on my computer and it's all green (i.e.
invoke test). - [x] I ran lint on my computer and there are no errors (i.e.
invoke lint). - [ ] I added new functions/classes and made them available on a second-level import, e.g.
compas.datastructures.Mesh. - [x] I have added tests that prove my fix is effective or that my feature works.
- [x] I have added necessary documentation (if appropriate)
Fixes compas-dev/compas#1493
Original prompt
This section details on the original issue you should resolve
<issue_title>More transformations of shapes</issue_title> <issue_description>Following up on compas-dev/compas#1489, it seems like using
.transform()or.transformed()in any subclass ofcompas.geometry.Shapewill result in thescalecomponent of the transformation to be ignored, because only the shape's frame is transformed.There's a commented out piece of code in
Boxthat seems to have attempted a fix to that, doing matrix decomposition of the transformation and then applying thescaleto the dimensions of the box, but for some reason is commented out and it's also not implemented in any of the other shapes (as far as I checked).As a first step, we should write unit tests that verify the transform and transformed results for each of the shapes in
compas.geometry(sub-classes ofShape). If these tests fail, it means the transformation is indeed not applying the scale component, so the second step would be to add the implementation for that, and hopefully get the unit tests to pass. </issue_description>Comments on the Issue (you are @copilot in this section)
Fixes compas-dev/compas#1493
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Codecov Report
:x: Patch coverage is 98.30508% with 1 line in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 62.35%. Comparing base (e34268a) to head (bc4bcf5).
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/compas/geometry/shapes/shape.py | 0.00% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #1494 +/- ##
==========================================
+ Coverage 62.26% 62.35% +0.09%
==========================================
Files 208 208
Lines 22452 22510 +58
==========================================
+ Hits 13979 14037 +58
Misses 8473 8473
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@copilot run the ruff linter (through invoke tasks) and fix all linting issues
@copilot run the ruff linter (through
invoketasks) and fix all linting issues
Fixed all ruff linting issues. Removed 3 unused variables from test file (commit 0a2be4d).
@copilot rebase on top of current main head
@copilot rebase on top of current
mainhead
Rebased on top of current main head (43cb6b6). Resolved merge conflict in cone.py where the new scale() method from main was integrated with the transform() method. All tests pass.
if we are re-adding these implementations, we should remove the frame transformation from the base class, and raise a NotImplementedError, like with the other transformation methods...
@copilot please also do this for PlanarSurface
@copilot please also do this for
PlanarSurface
Added transform() implementation to PlanarSurface with scale component handling and comprehensive tests (commit fd3dd45).
if we are re-adding these implementations, we should remove the frame transformation from the base class, and raise a
NotImplementedError, like with the other transformation methods...
Changed Shape.transform() to raise NotImplementedError for consistency with other transformation methods like scale() (commit fd3dd45).