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.

Sign up for GitHub

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

Jump to bottom

added integration tests for scoring modules #968

Merged
merged 13 commits into from
Sep 10, 2024
Merged

added integration tests for scoring modules #968

merged 13 commits into from
Sep 10, 2024

Conversation

mgiulini
Copy link
Contributor

@mgiulini mgiulini commented Aug 6, 2024 •
edited
Loading

You are about to submit a new Pull Request. Before continuing make sure you read the contributing guidelines and that you comply with the following criteria:

  • You have sticked to Python. Please talk to us before adding other programming languages to HADDOCK3
  • Your PR is about CNS
  • Your code is well documented: proper docstrings and explanatory comments for those tricky parts
  • You structured the code into small functions as much as possible. You can use classes if there is a (state) purpose
  • Your code follows our coding style
  • You wrote tests for the new code
  • tox tests pass. Run tox command inside the repository folder
  • -test.cfg examples execute without errors. Inside examples/ run python run_tests.py -b
  • PR does not add any dependencies, unless permission granted by the HADDOCK team
  • PR does not break licensing
  • Your PR is about writing documentation for already existing code 🔥
  • Your PR is about writing tests for already existing code :godmode:

Closes #967 by adding two simple integration tests for the CNS scoring modules. Simple checks on the output are performed. Due to the chaotic nature of the score calculation, the score attribute cannot be checked precisely.

@mgiulini mgiulini self-assigned this Aug 6, 2024
@mgiulini mgiulini added enhancement Enhancing an existing feature of adding a new one m|emscoring Related to emscoring module m|mdscoring mdscoring module labels Aug 6, 2024
VGPReys
VGPReys previously approved these changes Aug 6, 2024
View reviewed changes
Copy link
Contributor

@VGPReys VGPReys left a comment

Choose a reason for hiding this comment

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

Do we want to keep it separated like this into two distinct files?
Or do we want to have an integration test devoted to CNS scoring modules?
Because basically the two scripts are the same, with the only difference from the imports.

@mgiulini @VGPReys
Update integration_tests/test_emscoring.py
670e87a 
Co-authored-by: Victor Reys <132575181+VGPReys@users.noreply.github.com>
mgiulini and others added 6 commits August 7, 2024 09:37
@mgiulini @VGPReys
Update integration_tests/test_emscoring.py
c06d4e5 
Co-authored-by: Victor Reys <132575181+VGPReys@users.noreply.github.com>
@mgiulini @VGPReys
Update integration_tests/test_emscoring.py
db8dcc7 
Co-authored-by: Victor Reys <132575181+VGPReys@users.noreply.github.com>
@mgiulini @VGPReys
Update integration_tests/test_emscoring.py
62ffb63 
Co-authored-by: Victor Reys <132575181+VGPReys@users.noreply.github.com>
@mgiulini @VGPReys
Update integration_tests/test_mdscoring.py
bcbf408 
Co-authored-by: Victor Reys <132575181+VGPReys@users.noreply.github.com>
@mgiulini @VGPReys
Update integration_tests/test_mdscoring.py
f2c1d83 
Co-authored-by: Victor Reys <132575181+VGPReys@users.noreply.github.com>
@mgiulini @VGPReys
Update integration_tests/test_mdscoring.py
59c0a4d 
Co-authored-by: Victor Reys <132575181+VGPReys@users.noreply.github.com>
@mgiulini
Copy link
Contributor Author

mgiulini commented Aug 7, 2024

Do we want to keep it separated like this into two distinct files? Or do we want to have an integration test devoted to CNS scoring modules? Because basically the two scripts are the same, with the only difference from the imports.

i see the point, but would keep the two tests separate for the moment, to follow the current one module-one test pattern

@mgiulini mgiulini requested a review from VGPReys August 7, 2024 07:39
rvhonorato
rvhonorato requested changes Aug 7, 2024
View reviewed changes
integration_tests/test_emscoring.py Outdated Show resolved Hide resolved
integration_tests/test_emscoring.py Outdated Show resolved Hide resolved
integration_tests/test_mdscoring.py Outdated Show resolved Hide resolved
integration_tests/test_emscoring.py Outdated Show resolved Hide resolved
integration_tests/test_mdscoring.py Outdated Show resolved Hide resolved
@amjjbonvin
Copy link
Member

amjjbonvin commented Aug 8, 2024 via email

@mgiulini
Copy link
Contributor Author

mgiulini commented Aug 8, 2024

Exact coordinates might be a hard test as it might depend on the hardware where it is run… Or you have to perform a RMSD assessment and accept it should be within 0.? A

this is a very good idea Alex!

@mgiulini mgiulini mentioned this pull request Aug 13, 2024
Open
12 tasks
@rvhonorato rvhonorato merged commit e297ca6 into main Sep 10, 2024
4 checks passed
@rvhonorato rvhonorato deleted the scoring_integration_tests branch September 10, 2024 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rvhonorato rvhonorato rvhonorato approved these changes

@VGPReys VGPReys VGPReys approved these changes

Assignees

@mgiulini mgiulini

Labels
enhancement Enhancing an existing feature of adding a new one m|emscoring Related to emscoring module m|mdscoring mdscoring module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add integration tests for scoring modules
4 participants
@mgiulini @amjjbonvin @rvhonorato @VGPReys