Skip to content
This repository has been archived by the owner on Jan 26, 2024. It is now read-only.

Prevent atom alternative locations from ending up in the dataset #15

Merged
merged 10 commits into from
Nov 16, 2021

Conversation

cbaakman
Copy link
Collaborator

Some PDB files have alternative locations for certain atoms. These changes make sure that the atom with the highest occupancy is taken, rather than all atoms.

Tests added:

  • test that of 5MNH chain A residue 153 the atom C is taken, which has the highest occupancy.
  • test that 5MNH can be preprocessed without error
  • test that, if a feature does contain NaN values, the entry is not added to the preprocessed data.

Copy link
Collaborator

@rgayatri rgayatri left a comment

Choose a reason for hiding this comment

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

Thanks Coos, the changes look good.
But why is the build failing? Are there other value errors?

@cbaakman
Copy link
Collaborator Author

Thanks Coos, the changes look good. But why is the build failing? Are there other value errors?

I know, that was a surprise to me too. I ran each test individually, but not all at the same time. I'm checking it now..

@cbaakman
Copy link
Collaborator Author

There were indeed still several bugs that caused the tests to fail. The tests do pass now though.

@coveralls
Copy link

coveralls commented Nov 16, 2021

Pull Request Test Coverage Report for Build 1467968399

  • 65 of 65 (100.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.7%) to 71.257%

Totals Coverage Status
Change from base Build 1452743238: 0.7%
Covered Lines: 2370
Relevant Lines: 3326

💛 - Coveralls

@rgayatri
Copy link
Collaborator

There were indeed still several bugs that caused the tests to fail. The tests do pass now though.

Great! I'll merge this then.

@rgayatri rgayatri closed this Nov 16, 2021
@rgayatri rgayatri reopened this Nov 16, 2021
@rgayatri rgayatri merged commit 46e8c25 into main Nov 16, 2021
@cbaakman cbaakman deleted the nan-grid branch March 17, 2022 14:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants