Electrostatics / pdb2pqr

PDB2PQR - determining titration states, adding missing atoms, and assigning charges/radii to biomolecules.
http://www.poissonboltzmann.org/
Other
124 stars 34 forks source link

`main_driver` to return information and data structures. #198

Closed stefdoerr closed 3 years ago

stefdoerr commented 3 years ago

Hi, I guess this is a bit more of a controversial PR than the last ones. I'm trying to use pdb2pqr programmatically from the API instead of command line and I'd like to have access to some of data generated in the main_driver function. Since there is lots of logic happening inside main_driver I didn't want to copy-paste it into my code, since I would lose any updates you might do to that function. Therefore I'd rather have it return data related to it's computations and propka's which are not stored in the PQR or PDB outputs.

Also I save the insertion code from propka since the res_num is not a unique identifier and also the buriedness since it's helpful to check for issues in membrane proteins.

I'm open to suggestions if you want to return different data or you don't like the idea. Best, Stefan

stefdoerr commented 3 years ago

What about the tests not passing? I could not figure out the exact reason they were failing.

sobolevnrm commented 3 years ago

It was a problem with the tests not related to your code. I'll fix it in a separate issue at some point... my issue backlog is huge!

Besides, your code passed @intendo 's review -- and he's waaaay pickier than I am. :)

intendo commented 3 years ago

@stefdoerr the reason that 1 test fails is that the newly added key, pka_df was added to the results dictionary in the non_trivial function that gets returned.

This pka_df key does not exist if the args.clean flag is set in main_driver (e.g., during the tests). Since main_driver returns results["pka_df"] and it doesn't exist, the test results with an error, KeyError: 'pka_df'

I think one fix would be to insert the following line in main.py at line 763:

"pka_df": None,

NOTE: The fix above does allow the failing test to pass.

@sobolevnrm do you agree with the suggested fix?

Then you could submit a new pull request.

sobolevnrm commented 3 years ago

@intendo -- that sounds good to me.

intendo commented 3 years ago

@stefdoerr I went ahead and made the change, committed it, and all tests pass.

Thanks!