jensengroup / propka

PROPKA predicts the pKa values of ionizable groups in proteins and protein-ligand complexes based in the 3D structure.
http://propka.org
GNU Lesser General Public License v2.1
263 stars 58 forks source link

Handle KeyError in read_parameter_file #65

Closed speleo3 closed 4 years ago

speleo3 commented 4 years ago

Fixes the following error:

conda create -n propka python=3.8 pytest pandas numpy
conda activate propka
python setup.py install
cd /tmp
propka3 1ubq.pdb

Result:

KeyError: 'propka/home/thomas/.cache/Python-Eggs/propka-3.2.0-py3.8.egg-tmp/propka/propka.cfg'
orbeckst commented 4 years ago

Dumb question: under which circumstances should the try fail and use the except clause? I would have throught that for a normal installation, we will always take the bundled file.

speleo3 commented 4 years ago

@orbeckst good question, I don't know (I'm not familiar with the history of this code). The parameters file can be specified with the --parameters option, and the default value is already the absolute path from the pkg_resources lookup. Maybe this try/except should be the other way round, first try the path as-is, then look it up from resources as the fallback?

orbeckst commented 4 years ago

That would make a lot more sense if you could override it from the command line.

sobolevnrm commented 4 years ago

@speleo3 did pip install . work or does it generate the same error? Were you using the latest from https://github.com/Electrostatics/propka-3.1/tree/nathan/v3.2 ?

FWIW, I plan to nuke the https://github.com/Electrostatics/propka-3.1/tree/nathan/v3.2 fork after the 3.2 release is out.

speleo3 commented 4 years ago

@sobolevnrm it works with pip install . and also with python setup.py install --single-version-externally-managed. The difference is that those two options don't install an "egg" (which is a zip file) but a regular site-packages/propka directory.

Yes I'm using the latest nathan/v3.2

sobolevnrm commented 4 years ago

@speleo3 is it OK to delete the branch associated with this PR?