Skip to content

GEOPY-2794: Change reference of Plate from center-center to top-center#193

Merged
domfournier merged 8 commits intodevelopfrom
GEOPY-2794
Apr 13, 2026
Merged

GEOPY-2794: Change reference of Plate from center-center to top-center#193
domfournier merged 8 commits intodevelopfrom
GEOPY-2794

Conversation

@benk-mira
Copy link
Copy Markdown
Contributor

@benk-mira benk-mira commented Apr 9, 2026

GEOPY-2794 - Change reference of Plate from center-center to top-center

Copilot AI review requested due to automatic review settings April 9, 2026 22:16
@github-actions github-actions bot changed the title Geopy 2794 GEOPY-2794: Change reference of Plate from center-center to top-center Apr 9, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 9, 2026

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.52%. Comparing base (2162766) to head (5f6450d).
⚠️ Report is 9 commits behind head on develop.

Files with missing lines Patch % Lines
geoapps_utils/modelling/plates.py 92.30% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #193      +/-   ##
===========================================
+ Coverage    84.86%   85.52%   +0.66%     
===========================================
  Files           19       19              
  Lines         1163     1161       -2     
  Branches       151      151              
===========================================
+ Hits           987      993       +6     
+ Misses         136      129       -7     
+ Partials        40       39       -1     
Files with missing lines Coverage Δ
geoapps_utils/modelling/plates.py 76.19% <92.30%> (+6.95%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the plate modelling utilities to use a consistent unrotated bounding-box definition (dip extent anchored at the origin) and refactors geometry/masking to share that logic, with corresponding test updates.

Changes:

  • Added a bounding_box helper and refactored Plate.vertices and inside_plate to use it.
  • Adjusted inside_plate/make_plate test expectations to match the new origin/dip-extent convention.
  • Made PlateModel.easting/northing/elevation default to 0.0 and expanded docstrings.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
geoapps_utils/modelling/plates.py Introduces bounding_box, refactors geometry/masking around it, and changes PlateModel defaults/documentation.
tests/plates_test.py Adds coverage for bounding_box and updates masks/expectations for the new bounding-box convention.

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

Comment thread geoapps_utils/modelling/plates.py
Comment thread geoapps_utils/modelling/plates.py
Comment thread geoapps_utils/modelling/plates.py Outdated
Copy link
Copy Markdown
Contributor

@MatthieuCMira MatthieuCMira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems good to me.
I docstring missing.
Can you explain me why you need defualt values if in the model_validator you check for the values to be there?

Comment thread geoapps_utils/modelling/plates.py Outdated
Comment thread geoapps_utils/modelling/plates.py
domfournier
domfournier previously approved these changes Apr 13, 2026
Comment thread geoapps_utils/modelling/plates.py Outdated
@domfournier domfournier merged commit 50ea38e into develop Apr 13, 2026
19 checks passed
@domfournier domfournier deleted the GEOPY-2794 branch April 13, 2026 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants