Skip to content
178 changes: 127 additions & 51 deletions modules/pymol/commanding.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,19 @@
if True:
import _thread as thread
import urllib.request as urllib2
from io import FileIO as file
from io import FileIO as file, BytesIO

import builtins
import inspect
import glob
import shlex
import tokenize
from enum import Enum
from functools import wraps
from pathlib import Path
from textwrap import dedent
from typing import List
from typing import Tuple, Iterable, get_args, Optional, Union, Any, NewType, List, get_origin


import re
import os
Expand Down Expand Up @@ -600,45 +603,128 @@ def get_state_list(states_str):
states_list = sorted(set(map(int, output)))
return _cmd.delete_states(_self._COb, name, states_list)

class Selection(str):
pass


def _parse_bool(value: str):
if isinstance(value, str):
if value.lower() in ["yes", "1", "true", "on", "y"]:
return True
elif value.lower() in ["no", "0", "false", "off", "n"]:
return False
else:
raise Exception("Invalid boolean value: %s" % value)
elif isinstance(value, bool):
def _into_types(type, value):
if repr(type) == 'typing.Any':
return value
else:
raise Exception(f"Unsuported boolean flag {value}")

def _parse_list_str(value):
return shlex.split(value)

elif type is bool:
if isinstance(value, bool):
return value
elif isinstance(value, str):
if value.lower() in ["yes", "1", "true", "on", "y"]:
return True
elif value.lower() in ["no", "0", "false", "off", "n"]:
return False
elif isinstance(value, int):
if value in [0, 1]:
return bool(value)
else:
raise pymol.CmdException("Invalid boolean value: %s" % value)

def _parse_list_int(value):
return list(map(int, shlex.split(value)))
elif isinstance(type, Enum):
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition will never be true. type is a type annotation, not an instance of Enum. Use inspect.isclass(type) and issubclass(type, Enum) to check if the type is an Enum class.

Copilot uses AI. Check for mistakes.
if value in type:
return type(value)
else:
raise pymol.CmdException(f"Invalid value for enum {type.__name__}: {value}")

elif isinstance(type, builtins.type):
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is too broad and may incorrectly handle type annotations. Consider using more specific type checking logic or inspect.isclass() for better type validation.

Suggested change
elif isinstance(type, builtins.type):
elif inspect.isclass(type):

Copilot uses AI. Check for mistakes.
return type(value)

if origin := get_origin(type):
if not repr(origin).startswith('typing.') and issubclass(origin, tuple):
args = get_args(type)
new_values = []
for i, new_value in enumerate(shlex.split(value)):
new_values.append(_into_types(args[i], new_value))
return tuple(new_values)

elif origin == Union:
args = get_args(type)
found = False
for i, arg in enumerate(args):
try:
found = True
return _into_types(arg, value)
except:
found = False
if not found:
raise pymol.CmdException("Union was not able to cast %s" % value)

elif issubclass(list, origin):
args = get_args(type)
if len(args) > 0:
f = args[0]
else:
f = lambda x: x
return [f(i) for i in shlex.split(value)]

# elif value is None:
# origin = get_origin(type)
# if origin is None:
# return None
# else:
# return _into_types(origin)
# for arg in get_args(origin):
# return _into_types(get_args(origin), value)

elif isinstance(type, str):
return str(value)

raise pymol.CmdException(f"Unsupported argument type {type}")

def parse_documentation(func):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this function probably needs its own unit test as well. The API to me is not inherently clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is tested integrated with declare_command on test_declare_command_arg_docs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, but something like this probably deserves its own test too if possible. I'll make it optional, but I will likely add it later.

source = inspect.getsource(func)
tokens = tokenize.tokenize(BytesIO(source.encode('utf-8')).readline)
tokens = list(tokens)
comments = []
params = {}
i = -1
started = False
while True:
i += 1
if tokens[i].string == "def":
while tokens[i].string == "(":
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause an infinite loop if the opening parenthesis is found, as the condition never changes. The logic should find the opening parenthesis and then break or continue processing.

Suggested change
while tokens[i].string == "(":
if tokens[i].string == "(":

Copilot uses AI. Check for mistakes.
i += 1
started = True
continue
if not started:
continue
if tokens[i].string == "->":
break
if tokens[i].type == tokenize.NEWLINE:
break
if tokens[i].string == ")":
break
if tokens[i].type == tokenize.COMMENT:
comments.append(tokens[i].string)
continue
if tokens[i].type == tokenize.NAME and tokens[i+1].string == ":":
name = tokens[i].string
name_line = tokens[i].line
i += 1
while not (tokens[i].type == tokenize.NAME and tokens[i+1].string == ":"):
Comment on lines +709 to +713
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop may cause an IndexError when accessing tokens[i+1] if i reaches the end of the tokens list. Add bounds checking before accessing tokens[i+1].

Suggested change
if tokens[i].type == tokenize.NAME and tokens[i+1].string == ":":
name = tokens[i].string
name_line = tokens[i].line
i += 1
while not (tokens[i].type == tokenize.NAME and tokens[i+1].string == ":"):
if tokens[i].type == tokenize.NAME and (i+1 < len(tokens)) and tokens[i+1].string == ":":
name = tokens[i].string
name_line = tokens[i].line
i += 1
while not (tokens[i].type == tokenize.NAME and (i+1 < len(tokens)) and tokens[i+1].string == ":"):

Copilot uses AI. Check for mistakes.
if tokens[i].type == tokenize.COMMENT and tokens[i].line == name_line:
comments.append(tokens[i].string)
break
elif tokens[i].type == tokenize.NEWLINE:
break
i += 1
else:
i -= 3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I follow this line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me neither, I just placed it here because was needed. I'd need to debug to remember exactly, but seems to me that it's related to rewind the cursor i when no doc is found.

docs = ' '.join(c[1:].strip() for c in comments)
params[name] = docs
comments = []
return params

def _parse_list_float(value):
return list(map(float, shlex.split(value)))

def declare_command(name, function=None, _self=cmd):

if function is None:
name, function = name.__name__, name

# new style commands should have annotations
annotations = [a for a in function.__annotations__ if a != "return"]
if function.__code__.co_argcount != len(annotations):
raise Exception("Messy annotations")

# docstring text, if present, should be dedented
if function.__doc__ is not None:
function.__doc__ = dedent(function.__doc__).strip()

function.__doc__ = dedent(function.__doc__)

# Analysing arguments
spec = inspect.getfullargspec(function)
Expand All @@ -661,34 +747,24 @@ def inner(*args, **kwargs):

# It was called from command line or pml script, so parse arguments
if caller == _parser_filename:
kwargs = {**kwargs_, **kwargs, **dict(zip(args2_, args))}
kwargs = {**kwargs, **dict(zip(args2_, args))}
kwargs.pop("_self", None)
for arg in kwargs.copy():
if funcs[arg] == bool:
funcs[arg] = _parse_bool
elif funcs[arg] == List[str]:
funcs[arg] = _parse_list_str
elif funcs[arg] == List[int]:
funcs[arg] = _parse_list_int
elif funcs[arg] == List[float]:
funcs[arg] = _parse_list_float
else:
# Assume it's a literal supported type
pass
# Convert the argument to the correct type
kwargs[arg] = funcs[arg](kwargs[arg])
return function(**kwargs)
new_kwargs = {}
for var, type in funcs.items():
if var in kwargs:
value = kwargs[var]
new_kwargs[var] = _into_types(type, value)
return function(**new_kwargs)

# It was called from Python, so pass the arguments as is
else:
return function(*args, **kwargs)
inner.__arg_docs = parse_documentation(function)

name = function.__name__
_self.keyword[name] = [inner, 0, 0, ",", parsing.STRICT]
_self.kwhash.append(name)
_self.help_sc.append(name)
_self.keyword[name] = [inner, 0,0,',',parsing.STRICT]
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Inconsistent spacing around commas. Should be [inner, 0, 0, ',', parsing.STRICT] to match Python style conventions.

Suggested change
_self.keyword[name] = [inner, 0,0,',',parsing.STRICT]
_self.keyword[name] = [inner, 0, 0, ',', parsing.STRICT]

Copilot uses AI. Check for mistakes.
return inner


def extend(name, function=None, _self=cmd):

'''
Expand Down
Loading