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

first draft of solution to selaltloc bug #154

Closed
wants to merge 2 commits into from

Conversation

mgiulini
Copy link

@mgiulini mgiulini commented Feb 8, 2023

Closes #153 by adding an if-clause on the ' ' key in flush_resloc_occ function.

@mgiulini
Copy link
Author

mgiulini commented Feb 8, 2023

Two tests do not pass, namely:

pdb_selaltloc data/dummy_altloc.pdb
pdb_selaltloc data/dummy_altloc2.pdb

@joaomcteixeira could you please check them?

according to the old implementation the residue ILE 25 should be excluded, but in my opinion that's wrong.

Please check this PR carefully, as I didn't fully understand the quite complicated logic of the algorithm.

@joaomcteixeira
Copy link
Member

Thanks Marco,

Sorry to ask, but could you please add the test cases for the example we are trying to solve, along with some testing PDBs? Otherwise will be very difficult to review and test the code. pdb-tools is one of those projects we really need to go along with test-driven development; otherwise, we are lost because there are so many cases.

I will look into the detail of those tests you referred to, but those have been so inspected in the past, yet a bug could have gone along.

Cheers,

@mgiulini
Copy link
Author

mgiulini commented Feb 8, 2023

hi Joao, a reference case is already present in #153, I copied part of that file in the tests/data folder and implemented a new test.

Cheers,
Marco

@joaomcteixeira
Copy link
Member

Thanks, I will look at it asap.

@mgiulini
Copy link
Author

mgiulini commented Mar 7, 2023

hi @joaomcteixeira , do you have updates on this?

@joaomcteixeira
Copy link
Member

Not yet, sorry. Will try doing it asap.

@joaomcteixeira
Copy link
Member

Closing in favor of #156

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.

pdb_selaltloc removes residues with (only) alternate locations
2 participants