Pymol-Scripts / Pymol-script-repo

Collected scripts for Pymol
http://www.pymolwiki.org/index.php/Git_intro
445 stars 258 forks source link

elbow_angle.py: Fails silently when given `limit` residue doesn't exist. #108

Closed jaredsampson closed 4 years ago

jaredsampson commented 4 years ago

For a Fab PDB file with non-standard numbering, if either the limit_h or limit_l residue indicated doesn't exist, the elbow_angle command fails to print an elbow angle value without a warning or error message.

Ideally the script should check to make sure the indicated residue exists. Currently this is not implemented. However, at the very least, although the module docstring mentions there is no error checking for the limit residues or chain IDs, there should be some kind of error message if the elbow angle cannot be calculated.

Here is a small example of the issue:

reinitialize
fetch 3ghe, async=0
remove all and not (chain H+L and polymer.protein)
elbow_angle 3ghe
##     Elbow angle: 194 degrees
alter chain L and resi 107, resi='106A'
elbow_angle 3ghe
##     <no output>
jaredsampson commented 4 years ago

This appears to be caused by cmd.get_atom_coords() failing to return a value or raise an exception if the atom in the selection doesn't exist.

jaredsampson commented 4 years ago

Apparently in this case, a pymol.CmdException was being raised, but these are caught and handled by PyMOL, so there was no stacktrace, and since no error message was passed to the CmdException instance, nothing got printed, so the failure happened silently. The fix is to catch the CmdException, then raise a new CmdException with a descriptive error message. h/t @speleo3

pslacerda commented 4 years ago

Seems a good oportunity to ensure that unhundled exceptions get printed. A try except around the extended command could format the stacktrace in red and log into the QTextDocument output.

def extend(...):
    try:
        ...
    except Exception:
        traceback = ...
        print('<font color="red">' + traceback + '<font>')

As drawback it would print HTML tags at the console, it would be cumbersome in this case. So ideally it check if PyMOL is being used as library or GUI.


def extend(...):
    try:
        ...
    except Exception:
        traceback = ...
        if pymol._IS_LIBRARY:
            print(traceback)
        else:
            print('<font color="red">' + traceback + '<font>')

This is a suggestion that may be possible or not, but would avoid annoying bugs like this. Just thougts, please don't take too serious.

Another drawback is that some error messages are emited from C/C++ and because of this Python exceptions would be coloured and C/C++ errors would be regular. Qt slots could then be implemented to receive any messages, a slot for regular and other for errors. Or maybe simply enclose ErrMessage()s with <font color="red">.

jaredsampson commented 4 years ago

Thanks, @pslacerda, this looks interesting. I agree that I would prefer to see a traceback if something goes wrong.

Were you thinking this should be a module-level workaround? To me this seems more like a fix that should happen in PyMOL itself. @jarrettsjohnson indicated in schrodinger/pymol-open-source#74 that he is working on an automated error propagation mechanism which would handle these types of issues, so I suspect it may be worth holding off on this kind of global exception catching for the moment.

pslacerda commented 4 years ago

@jaredsampson I hadn't saw https://github.com/schrodinger/pymol-open-source/issues/74. And a signal/slot would also require that any messages be printed via signals or out of order could occur.

If I can request a feature is to errors be bolder than regular text.

Happy holidays!

speleo3 commented 4 years ago

Unhandled exceptions are already printed, unless they are CmdException or QuietExpection without a message. And in Incentive PyMOL the errors are colored (a small goody for financial supporters of the project :-)).

foo-traceback