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

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
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.

3 participants