-
Notifications
You must be signed in to change notification settings - Fork 108
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
Fix friday #1409
Fix friday #1409
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1409 +/- ##
==========================================
+ Coverage 61.39% 61.55% +0.16%
==========================================
Files 207 207
Lines 22275 22272 -3
==========================================
+ Hits 13675 13710 +35
+ Misses 8600 8562 -38 ☔ View full report in Codecov by Sentry. |
btw, @gonzalocasas the real changes to volmesh are in lines 1764 and onwards. the rest is just annoying code coverage feedabck because i added a few |
the section i commented out is a function that is not only broken , but also doesn't seem to make much sense. will remove it if there is no objection... |
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.
LGTM
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.
Tested is_halfface_on_boundary
and it works, I'm worried about the deletion of public methods, because this is not going to be a new major release
nbr_halfface = self._cell[cell][v][u] | ||
nbr_cell = self._plane[u][v][nbr_halfface] | ||
return None if nbr_cell is None else self._cell[nbr_cell][v][u] | ||
# def halfface_adjacent_halfface(self, halfface, halfedge): |
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.
This would be a breaking change
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.
sure but the method absolutely doesn't work and i don't even know what it is supposed to do...
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.
hehe ok, let's remove it, maybe adding a clear notice in the changelog
@@ -3108,37 +3111,6 @@ def halfface_opposite_halfface(self, halfface): | |||
nbr = self._plane[w][v][u] | |||
return None if nbr is None else self._cell[nbr][w][v] | |||
|
|||
def halfface_adjacent_halfface(self, halfface, halfedge): |
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.
This would be a breaking change, it shouldn't be included if this is going to be a patch release.
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.
it can't be that anyone has been using this because the description is nonsensical and the implementation doesn't work. we can leave it in, but i don't know how to fix it because i don't even know what it is supposed to do...
### Removed | ||
|
||
* Removed `VolMesh.halfface_adjacent_halfface` because of general nonsensicalness, and because it is (and probably always has been) completely broken. |
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.
😝
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
.