opensource/galaxy
3
0
Fork 0
mirror of https://github.com/galaxyproject/galaxy.git synced 2026-09-01 15:37:32 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix B023 warnings from flake8-bugbear 22.7.1

Browse Source 
The new flake8-bugbear 22.7.1 warns if a function defined inside a loop uses
a variable redefined in the loop, due to the late-binding closure gotcha:
https://docs.python-guide.org/writing/gotchas/#late-binding-closures

When possible/sensible, I've moved the function definition before the loop
(adding the variable(s) as parameters) which is clearly also a speed-up.

Most other cases are false alarms, e.g. if the function is used and
discarded within the loop iteration that defines it. In such cases, I've
annotated the offending lines with a `# noqa: B023`.

The only file where I've applied the suggested workaround of immediately
binding arguments is in `lib/galaxy/visualization/plugins/config_parser.py`

Also:
- Small refactorings, in particular to `scripts/apply_tags.py`
This commit is contained in:
Nicola Soranzo
2022-07-05 12:11:12 +01:00
parent 1a7934dee4
commit a382eacc58
14 changed files with 113 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/galaxy/dependencies/pinned-lint-requirements.txt
+1 -1
View File
@@ -1,6 +1,6 @@
attrs==21.4.0
flake8==4.0.1
flake8-bugbear==22.4.25
flake8-bugbear==22.7.1
importlib-metadata==4.2.0
mccabe==0.6.1
mypy==0.961
lib/galaxy/managers/markdown_parse.py
+18 -15
View File
@@ -64,21 +64,29 @@ WHITE_SPACE_ONLY_PATTERN = re.compile(r"^[\s]+$")
def validate_galaxy_markdown(galaxy_markdown, internal=True):
"""Validate the supplied markdown and throw an ValueError with reason if invalid."""
def invalid_line(template, line_no, **kwd):
if "line" in kwd:
kwd["line"] = kwd["line"].rstrip("\r\n")
raise ValueError("Invalid line %d: %s" % (line_no + 1, template.format(**kwd)))
def _validate_arg(arg_str, valid_args, line_no):
if arg_str is not None:
arg_name = arg_str.split("=", 1)[0].strip()
if arg_name not in valid_args:
invalid_line("Invalid argument to Galaxy directive [{argument}]", line_no, argument=arg_name)
expecting_container_close_for = None
last_line_no = 0
function_calls = 0
for (line, fenced, open_fence, line_no) in _split_markdown_lines(galaxy_markdown):
last_line_no = line_no
def invalid_line(template, **kwd):
if "line" in kwd:
kwd["line"] = line.rstrip("\r\n")
raise ValueError("Invalid line %d: %s" % (line_no + 1, template.format(**kwd)))
expecting_container_close = expecting_container_close_for is not None
if not fenced and expecting_container_close:
invalid_line(
"[{line}] is not expected close line for [{expected_for}]",
line_no,
line=line,
expected_for=expecting_container_close_for,
)
@@ -94,6 +102,7 @@ def validate_galaxy_markdown(galaxy_markdown, internal=True):
if not VALID_CONTAINER_END_PATTERN.match(line):
invalid_line(
"Invalid command close line [{line}] for [{expected_for}]",
line_no,
line=line,
expected_for=expecting_container_close_for,
)
@@ -109,7 +118,7 @@ def validate_galaxy_markdown(galaxy_markdown, internal=True):
if func_call_match:
function_calls += 1
if function_calls > 1:
invalid_line("Only one Galaxy directive is allowed per fenced Galaxy block (```galaxy)")
invalid_line("Only one Galaxy directive is allowed per fenced Galaxy block (```galaxy)", line_no)
container = func_call_match.group("container")
valid_args_raw = VALID_ARGUMENTS[container]
if isinstance(valid_args_raw, DynamicArguments):
@@ -118,13 +127,7 @@ def validate_galaxy_markdown(galaxy_markdown, internal=True):
first_arg_call = func_call_match.group("firstargcall")
def _validate_arg(arg_str):
if arg_str is not None:
arg_name = arg_str.split("=", 1)[0].strip()
if arg_name not in valid_args:
invalid_line("Invalid argument to Galaxy directive [{argument}]", argument=arg_name)
_validate_arg(first_arg_call)
_validate_arg(first_arg_call, valid_args, line_no)
rest = func_call_match.group("restargcalls")
while rest:
rest = rest.strip().split(",", 1)[1]
@@ -132,12 +135,12 @@ def validate_galaxy_markdown(galaxy_markdown, internal=True):
if not arg_match:
break
first_arg_call = arg_match.group("firstargcall")
_validate_arg(first_arg_call)
_validate_arg(first_arg_call, valid_args, line_no)
rest = arg_match.group("restargcalls")
continue
else:
invalid_line("Invalid embedded Galaxy markup line [{line}]", line=line)
invalid_line("Invalid embedded Galaxy markup line [{line}]", line_no, line=line)
# Markdown unrelated to Galaxy object containers.
continue
lib/galaxy/managers/workflows.py
+5 -5
View File
@@ -1107,12 +1107,12 @@ class WorkflowContentsManager(UsesAnnotations):
def callback(input, prefixed_name, **kwargs):
if isinstance(input, DataToolParameter) or isinstance(input, DataCollectionToolParameter):
data_input_names[prefixed_name] = True
multiple_input[prefixed_name] = input.multiple
data_input_names[prefixed_name] = True # noqa: B023
multiple_input[prefixed_name] = input.multiple # noqa: B023
if isinstance(input, DataToolParameter):
input_connections_type[input.name] = "dataset"
input_connections_type[input.name] = "dataset" # noqa: B023
if isinstance(input, DataCollectionToolParameter):
input_connections_type[input.name] = "dataset_collection"
input_connections_type[input.name] = "dataset_collection" # noqa: B023
visit_input_values(module.tool.inputs, module.state.inputs, callback)
# post_job_actions
@@ -1414,7 +1414,7 @@ class WorkflowContentsManager(UsesAnnotations):
def callback(input, prefixed_name, **kwargs):
if isinstance(input, DataToolParameter) or isinstance(input, DataCollectionToolParameter):
data_input_names[prefixed_name] = True
data_input_names[prefixed_name] = True # noqa: B023
# FIXME: this updates modules silently right now; messages from updates should be provided.
module.check_and_update_state()
lib/galaxy/metadata/__init__.py
+1 -1
View File
@@ -170,7 +170,7 @@ class PortableDirectoryMetadataGenerator(MetadataCollectionStrategy):
key = name
def _metadata_path(what):
return os.path.join(metadata_dir, f"metadata_{what}_{key}")
return os.path.join(metadata_dir, f"metadata_{what}_{key}") # noqa: B023
_initialize_metadata_inputs(
dataset, _metadata_path, tmp_dir, kwds, real_metadata_object=self.write_object_store_conf
lib/galaxy/model/dataset_collections/structure.py
+4 -4
View File
@@ -90,13 +90,13 @@ class Tree(BaseTree):
def _walk_collections(self, collection_dict):
for index, (_identifier, substructure) in enumerate(self.children):
def element(collection):
return collection[index]
def get_element(collection):
return collection[index] # noqa: B023
if substructure.is_leaf:
yield dict_map(element, collection_dict)
yield dict_map(get_element, collection_dict)
else:
sub_collections = dict_map(lambda collection: element(collection).child_collection, collection_dict)
sub_collections = dict_map(lambda collection: get_element(collection).child_collection, collection_dict)
for element in substructure._walk_collections(sub_collections):
yield element
lib/galaxy/model/store/__init__.py
+29 -30
View File
@@ -323,37 +323,36 @@ class ModelImportStore(metaclass=abc.ABCMeta):
def _import_datasets(self, object_import_tracker, datasets_attrs, history, new_history, job):
object_key = self.object_key
for dataset_attrs in datasets_attrs:
def handle_dataset_object_edit(dataset_instance, dataset_attrs):
if "dataset" in dataset_attrs:
assert self.import_options.allow_dataset_object_edit
dataset_attributes = [
"state",
"deleted",
"purged",
"external_filename",
"_extra_files_path",
"file_size",
"object_store_id",
"total_size",
"created_from_basename",
"uuid",
]
for attribute in dataset_attributes:
if attribute in dataset_attrs["dataset"]:
setattr(dataset_instance.dataset, attribute, dataset_attrs["dataset"][attribute])
self._attach_dataset_hashes(dataset_attrs["dataset"], dataset_instance)
self._attach_dataset_sources(dataset_attrs["dataset"], dataset_instance)
if "id" in dataset_attrs["dataset"] and self.import_options.allow_edit:
dataset_instance.dataset.id = dataset_attrs["dataset"]["id"]
if job:
dataset_instance.dataset.job_id = job.id
for dataset_attrs in datasets_attrs:
if "state" not in dataset_attrs:
self.dataset_state_serialized = False
def handle_dataset_object_edit(dataset_instance):
if "dataset" in dataset_attrs:
assert self.import_options.allow_dataset_object_edit
dataset_attributes = [
"state",
"deleted",
"purged",
"external_filename",
"_extra_files_path",
"file_size",
"object_store_id",
"total_size",
"created_from_basename",
"uuid",
]
for attribute in dataset_attributes:
if attribute in dataset_attrs["dataset"]:
setattr(dataset_instance.dataset, attribute, dataset_attrs["dataset"][attribute])
self._attach_dataset_hashes(dataset_attrs["dataset"], dataset_instance)
self._attach_dataset_sources(dataset_attrs["dataset"], dataset_instance)
if "id" in dataset_attrs["dataset"] and self.import_options.allow_edit:
dataset_instance.dataset.id = dataset_attrs["dataset"]["id"]
if job:
dataset_instance.dataset.job_id = job.id
if "id" in dataset_attrs and self.import_options.allow_edit and not self.sessionless:
dataset_instance = self.sa_session.query(getattr(model, dataset_attrs["model_class"])).get(
dataset_attrs["id"]
@@ -378,7 +377,7 @@ class ModelImportStore(metaclass=abc.ABCMeta):
value = replace_metadata_file(value, dataset_instance, self.sa_session)
setattr(dataset_instance, attribute, value)
handle_dataset_object_edit(dataset_instance)
handle_dataset_object_edit(dataset_instance, dataset_attrs)
else:
metadata_deferred = dataset_attrs.get("metadata_deferred", False)
metadata = dataset_attrs.get("metadata")
@@ -467,7 +466,7 @@ class ModelImportStore(metaclass=abc.ABCMeta):
# Otherwise, we will check for "file" information instead of dataset information - currently this includes
# "file_name", "extra_files_path".
if "dataset" in dataset_attrs:
handle_dataset_object_edit(dataset_instance)
handle_dataset_object_edit(dataset_instance, dataset_attrs)
else:
file_name = dataset_attrs.get("file_name")
if file_name:
@@ -888,7 +887,7 @@ class ModelImportStore(metaclass=abc.ABCMeta):
def attach_workflow_step(imported_object, attrs):
order_index = attrs["order_index"]
imported_object.workflow_step = workflow.step_by_index(order_index)
imported_object.workflow_step = workflow.step_by_index(order_index) # noqa: B023
for step_attrs in invocation_attrs["steps"]:
imported_invocation_step = model.WorkflowInvocationStep()
lib/galaxy/tool_util/verify/interactor.py
+7 -6
View File
@@ -303,21 +303,22 @@ class GalaxyInteractorApi:
def compare(val, expected):
if str(val) != str(expected):
msg = f"Dataset metadata verification for [{key}] failed, expected [{value}] but found [{dataset_value}]. Dataset API value was [{dataset}]."
raise Exception(msg)
raise Exception(
f"Dataset metadata verification for [{key}] failed, expected [{value}] but found [{dataset_value}]. Dataset API value was [{dataset}]." # noqa: B023
)
if isinstance(dataset_value, list):
value = str(value).split(",")
if len(value) != len(dataset_value):
msg = f"Dataset metadata verification for [{key}] failed, expected [{value}] but found [{dataset_value}], lists differ in length. Dataset API value was [{dataset}]."
raise Exception(msg)
raise Exception(
f"Dataset metadata verification for [{key}] failed, expected [{value}] but found [{dataset_value}], lists differ in length. Dataset API value was [{dataset}]."
)
for val, expected in zip(dataset_value, value):
compare(val, expected)
else:
compare(dataset_value, value)
except KeyError:
msg = f"Failed to verify dataset metadata, metadata key [{key}] was not found."
raise Exception(msg)
raise Exception(f"Failed to verify dataset metadata, metadata key [{key}] was not found.")
def wait_for_job(self, job_id, history_id=None, maxseconds=DEFAULT_TOOL_TEST_WAIT):
self.wait_for(lambda: self.__job_ready(job_id, history_id), maxseconds=maxseconds)
lib/galaxy/tools/data/__init__.py
+1 -1
View File
@@ -1039,7 +1039,7 @@ class RefgenieToolDataTable(TabularToolDataTable):
display_name = f"{genome}/{tagged_asset}@{digest}"
def _seek_key(key):
return rgc.seek(genome, asset, tag_name=tag, seek_key=key)
return rgc.seek(genome, asset, tag_name=tag, seek_key=key) # noqa: B023
template_dict = {
"__REFGENIE_UUID__": uuid,
lib/galaxy/tours/validate.py
+1 -1
View File
@@ -26,7 +26,7 @@ def main(argv=None):
tour_id = get_tour_id_from_path(tour_path)
def warn(msg):
print(f"Tour '{tour_id}' warning: {msg}")
print(f"Tour '{tour_id}' warning: {msg}") # noqa: B023
message = None
tour = None
lib/galaxy/visualization/plugins/config_parser.py
+5 -5
View File
@@ -308,27 +308,27 @@ class DataSourceParser:
if test_type == "isinstance":
# is test_attr attribute an instance of result
# TODO: wish we could take this further but it would mean passing in the datatypes_registry
def test_fn(o, result):
def test_fn(o, result, getter=getter):
return isinstance(getter(o), result)
elif test_type == "has_dataprovider":
# does the object itself have a datatype attr and does that datatype have the given dataprovider
def test_fn(o, result):
def test_fn(o, result, getter=getter):
return hasattr(getter(o), "has_dataprovider") and getter(o).has_dataprovider(result)
elif test_type == "has_attribute":
# does the object itself have attr in 'result' (no equivalence checking)
def test_fn(o, result):
def test_fn(o, result, getter=getter):
return hasattr(getter(o), result)
elif test_type == "not_eq":
def test_fn(o, result):
def test_fn(o, result, getter=getter):
return str(getter(o)) != result
else:
# default to simple (string) equilavance (coercing the test_attr to a string)
def test_fn(o, result):
def test_fn(o, result, getter=getter):
return str(getter(o)) == result
tests.append({"type": test_type, "result": test_result, "result_type": test_result_type, "fn": test_fn})
lib/galaxy/workflow/modules.py
+9 -7
View File
@@ -591,7 +591,7 @@ class SubWorkflowModule(WorkflowModule):
return
if is_runtime_value(value) and runtime_to_json(value)["__class__"] != "ConnectedValue":
input_name = "%d|%s" % (step.order_index, prefixed_name)
input_name = f"{step.order_index}|{prefixed_name}" # noqa: B023
inputs[input_name] = InputProxy(input, input_name)
visit_input_values(tool.inputs, tool_inputs.inputs, callback)
@@ -1116,7 +1116,7 @@ class InputParameterModule(WorkflowModule):
tool_inputs = module.tool.inputs # may not be set, but we're catching the Exception below.
def callback(input, prefixed_name, context, **kwargs):
if prefixed_name == connection.input_name and hasattr(input, "get_options"):
if prefixed_name == connection.input_name and hasattr(input, "get_options"): # noqa: B023
static_options.append(input.get_options(self.trans, {}))
visit_input_values(tool_inputs, module.state.inputs, callback)
@@ -1890,18 +1890,20 @@ class ToolModule(WorkflowModule):
replacement: Union[model.Dataset, NoReplacement] = NO_REPLACEMENT
dataset_instance: Optional[model.Dataset] = None
if iteration_elements and prefixed_name in iteration_elements:
dataset_instance = getattr(iteration_elements[prefixed_name], "dataset_instance", None)
if iteration_elements and prefixed_name in iteration_elements: # noqa: B023
dataset_instance = getattr(
iteration_elements[prefixed_name], "dataset_instance", None # noqa: B023
)
if isinstance(input, DataToolParameter) and dataset_instance:
# Pull out dataset instance (=HDA) from element and set a temporary element_identifier attribute
# See https://github.com/galaxyproject/galaxy/pull/1693 for context.
replacement = dataset_instance
temp = iteration_elements[prefixed_name]
temp = iteration_elements[prefixed_name] # noqa: B023
if hasattr(temp, "element_identifier") and temp.element_identifier:
replacement.element_identifier = temp.element_identifier # type: ignore[union-attr]
else:
# If collection - just use element model object.
replacement = iteration_elements[prefixed_name]
replacement = iteration_elements[prefixed_name] # noqa: B023
else:
replacement = progress.replacement_for_input(step, input_dict)
@@ -1912,7 +1914,7 @@ class ToolModule(WorkflowModule):
if getattr(dataset2, "extension", None) == "expression.json":
with open(dataset2.file_name) as f:
replacement = json.load(f)
found_replacement_keys.add(prefixed_name)
found_replacement_keys.add(prefixed_name) # noqa: B023
return replacement
lib/galaxy/workflow/refactor/execute.py
+1 -1
View File
@@ -276,7 +276,7 @@ class WorkflowRefactorExecutor:
return NO_REPLACEMENT
if value == target_value:
target_tool_inputs.append((step.order_index, input, prefixed_name))
target_tool_inputs.append((step.order_index, input, prefixed_name)) # noqa: B023
replace_tool_state = True
return runtime_to_json(ConnectedValue())
else:
scripts/apply_tags.py
+22 -29
View File
@@ -2,18 +2,17 @@
import sys
import time
from typing import List
from bioblend.galaxy import GalaxyInstance
class ApplyTagsHistory:
@classmethod
def __init__(self, galaxy_url, galaxy_api_key, history_id=None):
self.galaxy_url = galaxy_url
self.galaxy_api_key = galaxy_api_key
self.history_id = history_id
@classmethod
def read_galaxy_history(self):
"""
Read Galaxy's current history and inherit all the tags from a parent
@@ -38,7 +37,6 @@ class ApplyTagsHistory:
print("History id: %s" % update_history_id)
self.find_dataset_parents_update_tags(history, job, update_history_id)
@classmethod
def find_dataset_parents_update_tags(self, history, job, history_id):
"""
Operate on datasets for a particular history and recursively find parents
@@ -53,8 +51,8 @@ class ApplyTagsHistory:
print("Total datasets: %d. Updating their tags may take a while..." % len(all_datasets))
for dataset in all_datasets:
try:
if dataset["deleted"] is False and dataset["state"] == "ok":
parent_ids = list()
if not dataset["deleted"] and dataset["state"] == "ok":
parent_ids = []
child_dataset_id = dataset["id"]
own_tags[child_dataset_id] = dataset["tags"]
# get information about the dataset like the job id
@@ -91,46 +89,45 @@ class ApplyTagsHistory:
is_updated = self.propagate_tags(
history, history_id, parent_dataset_ids, dataset_id, parent_tags, own_tags
)
if is_updated is True:
if is_updated:
count_datasets_updated += 1
print("Tags of %d datasets updated" % count_datasets_updated)
@classmethod
def collect_parent_ids(self, datasets_inheritance_chain):
"""
Collect parent datasets for each dataset recursively
"""
def find_parent_recursive(dataset_id, recursive_parents):
if dataset_id in datasets_inheritance_chain:
# get parents of a dataset
dataset_parents = datasets_inheritance_chain[dataset_id]
# add all the parents to the recursive list
recursive_parents.extend(dataset_parents)
for parent in dataset_parents:
find_parent_recursive(parent, recursive_parents)
recursive_parent_ids = dict()
for item in datasets_inheritance_chain:
recursive_parents = list()
recursive_parents: List = []
def find_parent_recursive(dataset_id):
if dataset_id in datasets_inheritance_chain:
# get parents of a dataset
dataset_parents = datasets_inheritance_chain[dataset_id]
# add all the parents to the recursive list
recursive_parents.extend(dataset_parents)
for parent in dataset_parents:
find_parent_recursive(parent)
find_parent_recursive(item)
find_parent_recursive(item, recursive_parents)
# take unique parents
recursive_parent_ids[item] = list(set(recursive_parents))
return recursive_parent_ids
@classmethod
def collect_hash_tags(self, tags_list):
@staticmethod
def collect_hash_tags(tags_list):
"""
Collect only hash tags and exclude others if any
"""
return [tag for tag in tags_list if len(tag.split(":")) > 1]
@classmethod
def propagate_tags(self, history, current_history_id, parent_datasets_ids, dataset_id, parent_tags, own_tags):
"""
Propagate history tags from parent(s) to a child
"""
all_tags = list()
all_tags = []
for parent_id in parent_datasets_ids:
# collect all the tags from the parent
all_tags.extend(parent_tags[parent_id])
@@ -138,15 +135,12 @@ class ApplyTagsHistory:
all_tags = self.collect_hash_tags(all_tags)
self_tags = self.collect_hash_tags(own_tags[dataset_id])
# find unique tags from all parents
all_tags = set(all_tags)
all_tags_set = set(all_tags)
self_tags_set = set(self_tags)
is_same = all_tags == self_tags_set
# update tags if there are new tags from parents
if is_same is False:
is_subset = all_tags.issubset(self_tags_set)
if is_subset is False:
if all_tags_set != self_tags_set:
if not all_tags_set.issubset(self_tags_set):
# append the tags of the child itself
all_tags = list(all_tags)
all_tags.extend(self_tags)
# do a database update for the child dataset so that it reflects the tags from all parents
# take unique tags
@@ -155,7 +149,6 @@ class ApplyTagsHistory:
if __name__ == "__main__":
if len(sys.argv) < 3:
print("Usage: python apply_tags.py <galaxy_ip> <galaxy_api_key> <history_id as optional parameter>")
exit(1)
test/functional/test_toolbox.py
+9 -10
View File
@@ -93,6 +93,14 @@ def build_tests(
if key.startswith("TestForTool_"):
del G[key]
def make_test_method(tool_version, test_index, test_function_name):
def test_tool(self):
self.do_it(tool_version=tool_version, test_index=test_index)
test_tool.__name__ = test_function_name
return test_tool
tests_summary = galaxy_interactor.get_tests_summary()
for tool_id, tool_summary in tests_summary.items():
# Create a new subclass of ToolTestCase, dynamically adding methods
@@ -109,16 +117,7 @@ def build_tests(
count = version_summary["count"]
for i in range(count):
test_function_name = "test_tool_%06d" % all_versions_test_count
def make_test_method(tool_version, test_index):
def test_tool(self):
self.do_it(tool_version=tool_version, test_index=test_index)
test_tool.__name__ = test_function_name
return test_tool
test_method = make_test_method(tool_version, i)
test_method = make_test_method(tool_version, i, test_function_name)
test_method.__doc__ = "( %s ) > Test-%d" % (tool_id, all_versions_test_count + 1)
namespace[test_function_name] = test_method
namespace["tool_id"] = tool_id