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

fix bug in "from_json method" #248

Merged
merged 2 commits into from
Jul 1, 2024
Merged

Conversation

psauvan
Copy link
Member

@psauvan psauvan commented Jun 29, 2024

Load_step_file method was called before Settings class was updated.

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

Please include a summary of the changes and motivation for the changes

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

Load_step_file method was called before Settings class was updated.
@shimwell
Copy link
Collaborator

shimwell commented Jun 29, 2024

I think the load step file does not make use of information from the settings class or vice versa so perhaps the order does not matter?

the skip solids is not longer part of the settings class it is passed directly to the load_step method

    def load_step_file(
        self,
        filename: typing.Union[str, typing.Sequence[str]],
        skip_solids: typing.Sequence[int] = [],
    ):

@psauvan
Copy link
Member Author

psauvan commented Jun 29, 2024

Yes it does!
the compSolids parameter of the Settings class is used in load_step_file method.

@psauvan psauvan requested a review from shimwell June 29, 2024 22:01
Copy link
Collaborator

@shimwell shimwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point I should try to move that comp solid parameter if possible

@psauvan psauvan merged commit 8f0b2a1 into GEOUNED-org:dev Jul 1, 2024
9 checks passed
@psauvan psauvan deleted the from_json-bug branch July 1, 2024 06:54
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.

2 participants