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

points_to_coeffs fix - adding return #291

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

py1sl
Copy link
Contributor

@py1sl py1sl commented Dec 13, 2024

If you are a first-time contributor to GEOUNED, please read our contributing guidelines:
https://github.com/GEOUNED-org/GEOUNED/blob/main/CONTRIBUTING.md

Description

when using a plane defined by points an type error would be thrown for the coeff variable being none
On investigation this was found to be in geouned reverse in the mcnpinput in the Get_primitive_surfaces function a call is made to points_to_coeffs if the surface is a plane and is defined by points.
points_to_coeffs had no return so returned none.

simple fix to add a return coeffs to the points_to_coeffs function,

the points_to_coeffs could do with a refactor to be more pythonic and more readable

Fixes issue

Please link to any issues that this PR fixes

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

psauvan and others added 4 commits June 18, 2024 10:36
* adding version 1.3.0 to docs (GEOUNED-org#274)

* Update version_switcher.json

Adding the new version to the documentation.

* correction sphere additional plane

* Adding the mention of libmamba solver to installation instructions (GEOUNED-org#280)

* optimal bounbox to fix Torus bug

* optimal bounbox to fix Torus bug

* Torus envelope fix (partially) (GEOUNED-org#286)

* Update of the documentation layout (GEOUNED-org#285)

* update_of_docs

* test

* update 2

* update 3

* update 4

* test 5

* update 6

* update 7

* update 8

* test materials

* test again

* final

---------

Co-authored-by: Jonathan Shimwell <[email protected]>
Co-authored-by: Patrick Sauvan <[email protected]>
Co-authored-by: Aljaz Kolsek (F4E) <[email protected]>
@shimwell
Copy link
Collaborator

Hi @py1sl nice to see you over here. Looks like your PR brings in a few other commits but the intention is to add a single line. Looks like a useful addition but would it be possible to make the PR without the additional commits. Perhaps sync the dev branch in your fork and branch from that

    return coeff

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