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

Solvent class with multiple topologies #126

Merged
merged 5 commits into from
Jan 29, 2021
Merged

Solvent class with multiple topologies #126

merged 5 commits into from
Jan 29, 2021

Conversation

laumalo
Copy link
Collaborator

@laumalo laumalo commented Jan 22, 2021

@laumalo laumalo linked an issue Jan 22, 2021 that may be closed by this pull request
@codecov-io
Copy link

Codecov Report

Merging #126 (49a1200) into devel (eb02cd7) will increase coverage by 0.40%.
The diff coverage is 98.26%.

Copy link
Owner

@martimunicoy martimunicoy left a comment

Choose a reason for hiding this comment

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

I approve the merge with a minor comment on the testing. Excellent work @laumalo!!


# Check that merging both single topology dicitionaries we obtain the
# same dictionary that using multiple topologies
compare_dicts(merge_dicts(solvent_MAL_dict['SolventParameters'],
Copy link
Owner

Choose a reason for hiding this comment

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

This is a nice way to validate the Solvent object with multiple topologies. But I am missing one test that validates the output file that results from a Solvent with multiple topologies. Something similar that is done in the test_single_topology test comparing the outcome of the Solvent writer with a reference file.

@laumalo laumalo merged commit 666efe7 into devel Jan 29, 2021
@laumalo laumalo deleted the solvent_params branch January 29, 2021 11:21
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.

Solvent class compatible with multiple topologies
3 participants