Skip to content

[Review Needed]Pytest Style analysis/test_hydrogenbondautocorrel.py#1611

Merged
richardjgowers merged 3 commits intoMDAnalysis:developfrom
utkbansal:analysis-hba
Aug 14, 2017
Merged

[Review Needed]Pytest Style analysis/test_hydrogenbondautocorrel.py#1611
richardjgowers merged 3 commits intoMDAnalysis:developfrom
utkbansal:analysis-hba

Conversation

@utkbansal
Copy link
Member

Fixes #

Changes made in this Pull Request:

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

def O(self, u):
return u.atoms.select_atoms('name O')

# def setUp(self):
Copy link
Member

Choose a reason for hiding this comment

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

del this

@richardjgowers richardjgowers self-assigned this Aug 8, 2017
donors=N,
bond_type='continuous',
exclusions=self.excl_list,
exclusions=(np.array(range(len(H))), np.array(
Copy link
Contributor

Choose a reason for hiding this comment

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

Use np.arange instead of np.array(range(...))

return mda.Universe(TRZ_psf, TRZ)

@pytest.fixture()
def H(self, u):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you rename this fixture hydrogens, please? Let's avoid abusing single letter variable names.

return u.atoms.select_atoms('name Hn')

@pytest.fixture()
def N(self, u):
Copy link
Contributor

Choose a reason for hiding this comment

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

nitrogens?

return u.atoms.select_atoms('name N')

@pytest.fixture()
def O(self, u):
Copy link
Contributor

Choose a reason for hiding this comment

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

oxygens?

@richardjgowers richardjgowers merged commit 09d168c into MDAnalysis:develop Aug 14, 2017
@orbeckst orbeckst mentioned this pull request Aug 14, 2017
2 tasks
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