-
Notifications
You must be signed in to change notification settings - Fork 113
Fix bug in planar surface #1485
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
base: main
Are you sure you want to change the base?
Conversation
fix for #1301 |
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.
Pull Request Overview
This PR fixes a bug in the PlanarSurface.point_at
method where it was not properly scaling the UV coordinates by the surface dimensions, returning incorrect point locations.
- Fixed the
point_at
method to multiply UV coordinates by the surface's xsize and ysize - Added comprehensive test coverage for the corrected behavior
- Updated changelog to document the bug fix
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/compas/geometry/surfaces/planar.py | Fixed bug in point_at method to properly scale UV coordinates by surface dimensions |
tests/compas/geometry/test_surfaces_plane.py | Added test cases for surface sizing and mesh conversion functionality |
CHANGELOG.md | Added changelog entry for the bug fix |
tasks.py | Updated import path and removed unused grasshopper function |
Comments suppressed due to low confidence (1)
src/compas/geometry/surfaces/planar.py:1
- This commented-out code should be removed as it's no longer needed and creates clutter.
from __future__ import absolute_import
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
from compas_invocations2 import tests | ||
from compas_invocations2 import grasshopper | ||
from invoke import Collection | ||
from invoke.collection import Collection |
Copilot
AI
Oct 2, 2025
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.
Import should use from invoke import Collection
instead of the specific submodule path. This is the recommended import pattern for invoke.
from invoke.collection import Collection | |
from invoke import Collection |
Copilot uses AI. Check for mistakes.
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.
my editor suggests the opposite...
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1485 +/- ##
==========================================
+ Coverage 62.00% 62.04% +0.04%
==========================================
Files 208 208
Lines 22427 22427
==========================================
+ Hits 13905 13914 +9
+ Misses 8522 8513 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
only question is if the surface should be centered around the origin... |
although i was inclined to do so, it makes the UV space rather unintuitive... |
What type of change is this?
Checklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.CHANGELOG.md
file in theUnreleased
section under the most fitting heading (e.g.Added
,Changed
,Removed
).invoke test
).invoke lint
).compas.datastructures.Mesh
.