Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

change default compSolids behaviour #246

Merged
merged 3 commits into from
Jun 27, 2024

Conversation

alexvalentine94
Copy link
Collaborator

Description

The default behaviour for compSolids setting was set to True. This leads to solids in a single component being merged in the MCNP model cell definitions. The default should be false to respect the original structure tree.

Fixes issue

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 26, 2024

looks like the code is attempting to append to a bool somewhere and this is causing the tests to fail

this does not look like it is introduced by the PR but rather a part of the code that was not previously being executed in the CI.

looks like the error is on
geouned/GEOUNED/void/void_box_class.py line 351 in get_void_complementary which calls complementary.append(compSeq)

I guess complementary is the bool and is not appendable

@psauvan
Copy link
Member

psauvan commented Jun 27, 2024

I have fixed the problem with the boolean value, and updated dev branch (#247).
I rerun the tests but they still fail, it seems the tests are using the original source.

@alexvalentine94
Copy link
Collaborator Author

I have merged your fix Patrick, thanks for pointing to this. Tests passing now :)

@psauvan psauvan merged commit c14ad1d into GEOUNED-org:dev Jun 27, 2024
9 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants