Fix for #203, errors in simplify full #206
Conversation
There was a problem hiding this comment.
To be consistent, let the name of the variable in lines 442 and 481 be cut_box and rename the function in lines 463, 475 and 518 to divide_box
There was a problem hiding this comment.
Fair enough.
I've made this change. I also noticed that the same issue with a variable sharing the function name cut_box appears in GEOReverse as well, so I've made the same changes there.
| @@ -439,7 +439,7 @@ def point_inside(solid): | |||
| return point | |||
|
|
|||
| cut_line = 32 | |||
| cut_box = 2 | |||
| cutbox = 2 | |||
There was a problem hiding this comment.
| cutbox = 2 | |
| cut_box = 2 |
src/geouned/GEOUNED/core.py
Outdated
| Box = UF.get_box(c) | ||
| CT = build_c_table_from_solids(Box, (c.Surfaces, Surfs), option="full") | ||
| Box = UF.get_box(c,self.options) | ||
| CT = build_c_table_from_solids(Box, (c.Surfaces, Surfs), option="full",options=self.options) |
There was a problem hiding this comment.
| CT = build_c_table_from_solids(Box, (c.Surfaces, Surfs), option="full",options=self.options) | |
| CT = build_c_table_from_solids(Box, (c.Surfaces, Surfs),options=self.options) |
here I would prefer to pass only the self.options class as argument.
Also to avoid mistake the option attribute name of the self.options class should be changed from option to simplification_mode
There was a problem hiding this comment.
Unless I'm missing something, it doesn't look like just self.options will work. I believe simplify is contained in self.settings.simplify.
There was a problem hiding this comment.
Yes, you are right. The quick review made me make a mistake.
|
I believe I've made the requested changes. I also found a few other instances of the cut_box variable/function name bug in GEOReverse. Those have been updated. |
Add missing space to call to UF.get_box()
|
My apologies, there was a missing space in the call to UF.get_box, so it failed the formatting check. Everything else had passed. This has been remedied now. |
* Patch for #203 * Responding to comments on #206 * Update core.py Add missing space to call to UF.get_box() * Replace all get_box calls to have options.enlargeBox, change get_box to refer only to enlargeBox instead of options.enlargeBox --------- Co-authored-by: AlvaroCubi <55387701+AlvaroCubi@users.noreply.github.com> Co-authored-by: Grammer, Kyle <grammerkb@ornl.gov>
* Patch for #203 * Responding to comments on #206 * Update core.py Add missing space to call to UF.get_box() * Replace all get_box calls to have options.enlargeBox, change get_box to refer only to enlargeBox instead of options.enlargeBox --------- Co-authored-by: AlvaroCubi <55387701+AlvaroCubi@users.noreply.github.com> Co-authored-by: Grammer, Kyle <grammerkb@ornl.gov>
Description
Two calls in core.py to get_box and build_c_table_from_solids are missing a required "options" parameter. This has been added as "self.options".
The function point_inside in boolean_solids.py has a variable named "cut_box". This is the same name as a function in boolean_solids.py, cut_box(). The variable has the underscore removed in this function.
Fixes issue
Fixes #203
Checklist