Skip to content

#138 Refactoring and Testing of the Boundary Class#150

Open
JMorado wants to merge 25 commits intomasterfrom
138-boundary-class-refactor
Open

#138 Refactoring and Testing of the Boundary Class#150
JMorado wants to merge 25 commits intomasterfrom
138-boundary-class-refactor

Conversation

@JMorado
Copy link
Copy Markdown
Collaborator

@JMorado JMorado commented Aug 30, 2023

This PR introduces the following modifications, addressing issue #138:

  • Restructured the Boundary class for improved functionality.
  • Implemented unit and functional tests for the Boundary class, which is defined within the nemo_bdy_gen_c.py file. The original version of the Boundary class can be found in the nemo_bdy_gen_c_old.py file.

@JMorado JMorado requested review from jdha and jpolton August 30, 2023 11:51
Comment thread src/pybdy/nemo_bdy_gen_c.py Outdated
Comment thread src/pybdy/nemo_bdy_gen_c.py Outdated
Copy link
Copy Markdown
Collaborator

@jdha jdha left a comment

Choose a reason for hiding this comment

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

just syntax updates to be consistent with previous code

@JMorado
Copy link
Copy Markdown
Collaborator Author

JMorado commented Dec 20, 2023

@jdha, the docstrings are now consistent with the format used throughout the rest of the codebase.

Additionally, it appears that updating the cookiecutter template using make template-update is necessary. Any merging conflicts arising from this update should be resolved to ensure the successful completion of the pre-commit step in the CI workflow. In spite of that, I believe this PR is ready for merging, as this update is a separate issue.

@JMorado JMorado requested a review from jdha December 20, 2023 16:33
Copy link
Copy Markdown
Collaborator Author

@JMorado JMorado left a comment

Choose a reason for hiding this comment

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

Ready.

@rdPatmore
Copy link
Copy Markdown
Contributor

This was never merged? Can we get these changes incorporated?

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