Skip to content
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

Simplify error #211

Merged
merged 7 commits into from
Jun 7, 2024
Merged

Simplify error #211

merged 7 commits into from
Jun 7, 2024

Conversation

KBGrammer
Copy link
Contributor

Description

The get_box function in functions.py took the parameters comp and options, and used only options.enlargeBox. The previous fix in core.py for the call to this function passed self.options.enlargeBox. cut_box has been changed to take the parameters comp and enlargeBox.

An additional call to cut_box is in cell_definition.py that was missing the second parameter. The call here has been replaced with changed from only "m" to "m" and options.enlargeBox.

The result of the first issue is that simplify = "full" fails.

Fixes issue

Previous fix to #203 was not sufficient. This should be.

Checklist

  • I'm making a PR from a feature branch on my fork into GEOUNED-org/GEOUNED/dev branch
  • I have followed PEP8 style guide for Python or run a formatter such as black or ruff format on my code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

@shimwell
Copy link
Collaborator

shimwell commented Jun 5, 2024

These three line changes look good to me.
Thanks for another PR @KBGrammer
LGTM

@KBGrammer
Copy link
Contributor Author

I think this is ready to merge and should not require another attempt (famous last words).

@psauvan psauvan merged commit a3fd78f into GEOUNED-org:dev Jun 7, 2024
9 checks passed
psauvan pushed a commit that referenced this pull request Jun 17, 2024
* 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>
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