Pytest Style: test_selections.py #1471
Merged
Conversation
|
see minor comments, otherwise looking good |
| class _SelectionWriter(TestCase): | ||
| __test__ = False | ||
| class _SelectionWriter(object): | ||
| # __test__ = False |
orbeckst
Jul 11, 2017
Member
remove commented out line
remove commented out line
| @@ -189,7 +188,7 @@ def spt2array(line): | |||
|
|
|||
|
|
|||
| class TestSelectionWriter_Jmol(_SelectionWriter): | |||
| __test__ = True | |||
| # __test__ = True | |||
orbeckst
Jul 11, 2017
Member
remove comment (I assume that the test still gets executed?)
remove comment (I assume that the test still gets executed?)
|
Wouldn't this test suite benefit from parametrizing the base class instead of subclassing it? |
| @@ -100,7 +99,6 @@ def lines2one(lines): | |||
|
|
|||
|
|
|||
| class TestSelectionWriter_Gromacs(_SelectionWriter): | |||
kain88-de
Jul 11, 2017
Member
why are you using the composition here? We wanted to Avoid mixing classes.
why are you using the composition here? We wanted to Avoid mixing classes.
|
@jbarnoud @kain88-de |
|
Isn't this a case similar to the coordinates tests? |
|
Hum... They indeed differ more than I initially thought, especially the gromacs one. @kain88-de ? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Fixes #
Changes made in this Pull Request:
PR Checklist