Fix critical bug in python 3.8+ versions with temporary file creation

[stefdoerr]

I have to apologize for this in advance because it must have been me who introduced the bug when trying to get rid of the temporary files.

I should not have fed an unclosed file to propka. Seems like in python 3.8/3.9 it didn't finish writing all lines and gave different predictions (since whole residues were missing from the end). For some reason 3.6/3.7 worked ok so I picked it up in our CI when the new python versions were failing to pass. Since our CI didn't run because we were missing a conda release of pdb2pqr I didn't see this earlier. This should not happen hopefully now that I make my own bugfix releases more frequently.

[stefdoerr]

@intendo just to make sure, where should I add tests for this? in propka_test.py?

[intendo]

@stefdoerr I think so. Hopefully, you can just add an input_pdb to the list of pdbs in the pytest.mark.parametrize section.

[codecov[bot]]

Codecov Report

Merging #298 (dcf2080) into master (d30def3) will decrease coverage by 0.24%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #298      +/-   ##
==========================================
- Coverage   65.29%   65.04%   -0.25%     
==========================================
  Files          30       30              
  Lines        7779     7781       +2     
==========================================
- Hits         5079     5061      -18     
- Misses       2700     2720      +20     

Impacted Files	Coverage Δ
pdb2pqr/main.py	79.50% <100.00%> (+0.11%)	:arrow_up:
pdb2pqr/hydrogens/structures.py	80.28% <0.00%> (-2.82%)	:arrow_down:
pdb2pqr/aa.py	80.42% <0.00%> (+0.47%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d30def3...dcf2080. Read the comment docs.

[stefdoerr]

I am a bit curious as to why this was not caught by your tests because you also run 3.6-3.9 python versions. In theory it should have but maybe something is done differently compared to our testing.

[stefdoerr]

Actually the reason is that I think you are not testing the pKa values produced. In most of my PDBs the pKa's varied by around ~1.x and by chance the last residues were not getting protonated anyway so the resulting PDBs were identical. But I generate also a report of all the pKa's which is where I caught the issue.

[stefdoerr]

Ok, I added tests for pKa values predicted for groups, so there should be no more regressions on that part. If you see the pre-last commit the tests fail (I reverted my original fix to demonstrate that) and in the last commit where I apply the fix the tests pass again. In the pre-last commit you can see the tests failing because 3.9 python was missing a group which exists in the test CSV file (because not the whole file was written). Once I fix the file writing the group reappears and tests pass.

[sobolevnrm]

Actually the reason is that I think you are not testing the pKa values produced. In most of my PDBs the pKa's varied by around ~1.x and by chance the last residues were not getting protonated anyway so the resulting PDBs were identical. But I generate also a report of all the pKa's which is where I caught the issue.

I don't understand: what pKa values are you using in your tests? Does this commit's tests use the pKa values from previous versions' results?

[stefdoerr]

The csv test files have the fixed pKa values.

Here you can see the diff for 5DV8 which is the only one which fails in python 3.9 though. Left: before bugfix. Right: after bugfix. As you can see there is also a GLY 696 appearing after the bugfix which didn't exist before

[stefdoerr]

I didn't realize I should first commit test files with the broken version. I understood that I should commit correct test files to show how the master breaks and then commit the fix

[stefdoerr]

In commit a3fe87b you can see this error in the logs: Which crashed the tests in python 3.9

[sobolevnrm]

I didn't realize I should first commit test files with the broken version. I understood that I should commit correct test files to show how the master breaks and then commit the fix

Yes, that's the preferred approach but your explanation makes sense. Thank you very much!

[intendo]

@stefdoerr Thanks for your help fixing this problem