Max distance to joint rules #385
Conversation
There was a problem hiding this comment.
Left a few comments but look good all in all.
One thing though, of all the changes, this one has the most potential of breaking something that used to work. so the correctness here will have to be validated by @jonashaldemann and the teaching team
src/compas_timber/design/workflow.py
Outdated
| def comply(self, elements, model_max_distance=1e-6): | ||
| """Returns True if the given elements comply with this DirectRule. | ||
| only checks if the distance between the centerlines of the elements is less than the max_distance. | ||
| allows joint topology overrides. | ||
| """ | ||
| if self.max_distance: | ||
| max_distance = self.max_distance | ||
| else: | ||
| max_distance = model_max_distance |
There was a problem hiding this comment.
use compas.tolerance.TOL here instead:
def comply(self, elements, model_max_distance=None):
model_max_distance = model_max_distance or TOL.absolute
if self.max_distance is not None:
max_distance = self.max_distance
else:
max_distance = model_max_distance
There was a problem hiding this comment.
To control the max_distance through Joint rules definitely makes sense to me, because until now we had to set it globally, and this was a bit too inflexible. Did you do some tests with a few Structures in Grasshopper?
There was a problem hiding this comment.
To control the max_distance through Joint rules definitely makes sense to me, because until now we had to set it globally, and this was a bit too inflexible. Did you do some tests with a few Structures in Grasshopper?
I did some testing in GH, but the structures were not very complex. I added a bunch of unit tests, however.
There was a problem hiding this comment.
@chenkasirer @jonashaldemann I sent an invite for testing on Monday. I hope you guys are available. Maybe we can have a look at this then and then hopefully merge it for the big release.
closes #182
This adds the
max_distanceargument to JointRule subclasses and GH components, allowing the max distance to be set for each JointRule.What type of change is this?
Checklist
Put an
xin 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.mdfile in theUnreleasedsection under the most fitting heading (e.g.Added,Changed,Removed).invoke test).invoke lint).compas_timber.datastructures.Beam.