Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up use of mutable defaults under lib/ramble/ramble #835

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions lib/ramble/ramble/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,10 @@ def experiment_log_file(self, logs_dir):
"""Returns an experiment log file path for the given logs directory"""
return os.path.join(logs_dir, self.expander.experiment_namespace) + ".out"

def get_pipeline_phases(self, pipeline, phase_filters=["*"]):
def get_pipeline_phases(self, pipeline, phase_filters=None):
if phase_filters is None:
phase_filters = ["*"]

self.build_modifier_instances()
self.build_phase_order()

Expand Down Expand Up @@ -626,14 +629,16 @@ def run_phase(self, pipeline, phase, workspace):
phase_func(workspace, app_inst=self)
self._phase_times[phase] = time.time() - start_time

def print_phase_times(self, pipeline, phase_filters=["*"]):
def print_phase_times(self, pipeline, phase_filters=None):
"""Print phase execution times by pipeline phase order

Args:
pipeline (str): Name of pipeline to print timing information for
phase_filters (list(str)): Filters to limit phases to print
phase_filters (list(str) | None): Filters to limit phases to print
"""
logger.msg("Phase timing statistics:")
if phase_filters is None:
phase_filters = ["*"]
for phase in self.get_pipeline_phases(pipeline, phase_filters=phase_filters):
# Set default time to 0.0 s, to prevent KeyError from skipped phases
if phase not in self._phase_times:
Expand Down
8 changes: 6 additions & 2 deletions lib/ramble/ramble/cmd/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,13 @@ def __init__(
prog,
out=None,
aliases=False,
documented_commands=[],
rst_levels=["-", "-", "^", "~", ":", "`"],
documented_commands=None,
rst_levels=None,
):
if documented_commands is None:
documented_commands = []
if rst_levels is None:
rst_levels = ["-", "-", "^", "~", ":", "`"]
out = sys.stdout if out is None else out
super().__init__(prog, out, aliases, rst_levels)
self.documented = documented_commands
Expand Down
2 changes: 1 addition & 1 deletion lib/ramble/ramble/cmd/mirror.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def setup_parser(subparser):
def mirror_add(args):
"""Add a mirror to Ramble."""
url = url_util.format(args.url)
ramble.mirror.add(args.name, url, args.scope, args)
ramble.mirror.add(args.name, url, args.scope)


def mirror_remove(args):
Expand Down
12 changes: 9 additions & 3 deletions lib/ramble/ramble/expander.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ def define_value(
allow_passthrough=True,
expansion_func=str,
evaluation_func=eval,
no_expand_vars=set(),
used_vars=set(),
no_expand_vars=None,
used_vars=None,
):
"""Define the value for this node.

Expand All @@ -170,6 +170,10 @@ def define_value(
evaluation_func (func): function to use for evaluating math of strings
no_expand_vars (set): set of variable names that should never be expanded
"""
if no_expand_vars is None:
no_expand_vars = set()
if used_vars is None:
used_vars = set()
if self.contents is not None:
parts = []
last_idx = 0
Expand Down Expand Up @@ -336,7 +340,9 @@ class Expander:

_ast_dbg_prefix = "EXPANDER AST:"

def __init__(self, variables, experiment_set, no_expand_vars=set()):
def __init__(self, variables, experiment_set, no_expand_vars=None):
if no_expand_vars is None:
no_expand_vars = set()

self._keywords = ramble.keywords.keywords

Expand Down
4 changes: 2 additions & 2 deletions lib/ramble/ramble/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ class Filters:

def __init__(
self,
phase_filters=ALL_PHASES,
phase_filters=None,
include_where_filters=None,
exclude_where_filters=None,
tags=None,
):
"""Create a new filter instance"""

self.phases = phase_filters
self.phases = ALL_PHASES if phase_filters is None else phase_filters
self.include_where = None
self.exclude_where = None
self.tags = set()
Expand Down
18 changes: 12 additions & 6 deletions lib/ramble/ramble/graphs.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,22 @@ def _make_editable(self):
self._sorted = None
self._prepared = False

def update_graph(self, node, dep_nodes=[], internal_order=False):
def update_graph(self, node, dep_nodes=None, internal_order=False):
"""Update the graph with a new node and / or new dependencies.

Given a node, and list of dependencies, define new edges in the
graph. If the node is new, also construct a new phase node.

Args:
node (GraphNode): Node to inject or modify
dep_nodes (list(GraphNode)): List of nodes that are dependencies
dep_nodes (list(GraphNode) | None): List of nodes that are dependencies
internal_order (Boolean): True to process internal dependencies,
False to skip

"""

if dep_nodes is None:
dep_nodes = []
self._make_editable()
self.add_node(node)
self.define_edges(node, dep_nodes, internal_order=internal_order)
Expand All @@ -68,19 +70,21 @@ def add_node(self, node):
if node not in self.adj_list:
self.adj_list[node] = set()

def define_edges(self, node, dep_nodes=[], internal_order=False):
def define_edges(self, node, dep_nodes=None, internal_order=False):
"""Define graph edges

Process dependencies, and internal orderings (inside the node object)
to define new graph edges.

Args:
node (GraphNode): Node to inject or modify
dep_nodes (list(GraphNode)): List of nodes that are dependencies
dep_nodes (list(GraphNode) | None): List of nodes that are dependencies
internal_order (Boolean): True to process internal dependencies,
False to skip
"""

if dep_nodes is None:
dep_nodes = []
for dep in dep_nodes:
if dep.key not in self.node_definitions:
self.node_definitions[dep.key] = dep
Expand Down Expand Up @@ -198,19 +202,21 @@ def add_node(self, node, obj_inst=None):

super().add_node(node)

def update_graph(self, phase_name, dependencies=[], internal_order=False, obj_inst=None):
def update_graph(self, phase_name, dependencies=None, internal_order=False, obj_inst=None):
"""Update the graph with a new phase and / or new dependencies.

Given a phase name, and list of dependencies, define new edges in the
graph. If the phase is new, also construct a new phase node.

Args:
phase_name (str): Name of the phase to inject or modify
dependencies (list(str)): List of phase names to inject dependencies on
dependencies (list(str) | None): List of phase names to inject dependencies on
internal_order (Boolean): True to process internal dependencies,
False to skip
obj_inst (object): Application or modifier instance to extract phase function from
"""
if dependencies is None:
dependencies = []
if self._prepared:
del self._sorted
self._sorted = None
Expand Down
4 changes: 3 additions & 1 deletion lib/ramble/ramble/keywords.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,11 @@ class Keywords:
specific inputs to further configure the experiment.
"""

def __init__(self, extra_keys={}):
def __init__(self, extra_keys=None):
# Merge in additional Keys:
self.keys = default_keys.copy()
if extra_keys is None:
extra_keys = {}
self.update_keys(extra_keys)

def copy(self):
Expand Down
6 changes: 4 additions & 2 deletions lib/ramble/ramble/language/application_language.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,18 @@ def _execute_workload(app):


@application_directive("workload_groups")
def workload_group(name, workloads=[], mode=None, **kwargs):
def workload_group(name, workloads=None, mode=None, **kwargs):
"""Adds a workload group to this application

Defines a new workload group that can be used within the context of its
application.

Args:
name (str): The name of the group
workloads (list(str)): A list of workloads to be grouped
workloads (list(str) | None): A list of workloads to be grouped
"""
if workloads is None:
workloads = []

def _execute_workload_groups(app):
if mode == "append":
Expand Down
35 changes: 22 additions & 13 deletions lib/ramble/ramble/language/shared_language.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def figure_of_merit(
group_name,
log_file="{log_file}",
units="",
contexts=[],
contexts=None,
fom_type: FomType = FomType.UNDEFINED,
):
"""Adds a figure of merit to track for this object
Expand All @@ -100,9 +100,9 @@ def figure_of_merit(
fom_regex (str): A regular expression using named groups to extract the FOM
group_name (str): The name of the group that the FOM should be pulled from
units (str): The units associated with the FOM
contexts (list(str)): List of contexts (defined by
figure_of_merit_context) this figure of merit
should exist in.
contexts (list(str) | None): List of contexts (defined by
figure_of_merit_context) this figure of merit
should exist in.
fom_type: The type of figure of merit
"""

Expand All @@ -112,7 +112,7 @@ def _execute_figure_of_merit(obj):
"regex": fom_regex,
"group_name": group_name,
"units": units,
"contexts": contexts,
"contexts": [] if contexts is None else contexts,
"fom_type": fom_type,
}

Expand Down Expand Up @@ -276,7 +276,7 @@ def _execute_success_criteria(obj):

@shared_directive("builtins")
def register_builtin(
name, required=True, injection_method="prepend", depends_on=[], dependents=[]
name, required=True, injection_method="prepend", depends_on=None, dependents=None
):
"""Register a builtin

Expand Down Expand Up @@ -326,11 +326,16 @@ def example_builtin(self):
required (boolean): Whether the builtin will be auto-injected or not
injection_method (str): The method of injecting the builtin. Can be
'prepend' or 'append'
depends_on (list(str)): The names of builtins this builtin depends on
(and must execute after).
dependents (list(str)): The names of builtins that should come
after the current one.
depends_on (list(str) | None): The names of builtins this builtin depends on
(and must execute after).
dependents (list(str) | None): The names of builtins that should come
after the current one.
"""
if depends_on is None:
depends_on = []
if dependents is None:
dependents = []

supported_injection_methods = ["prepend", "append"]

def _store_builtin(obj):
Expand All @@ -355,7 +360,7 @@ def _store_builtin(obj):


@shared_directive("phase_definitions")
def register_phase(name, pipeline=None, run_before=[], run_after=[]):
def register_phase(name, pipeline=None, run_before=None, run_after=None):
"""Register a phase

Phases are portions of a pipeline that will execute when
Expand All @@ -370,9 +375,13 @@ def register_phase(name, pipeline=None, run_before=[], run_after=[]):
Args:
name (str): The name of the phase. Phases are functions named '_<phase>'.
pipeline (str): The name of the pipeline this phase should be registered into.
run_before (list(str)): A list of phase names this phase should run before
run_after (list(str)): A list of phase names this phase should run after
run_before (list(str) | None): A list of phase names this phase should run before
run_after (list(str) | None): A list of phase names this phase should run after
"""
if run_before is None:
run_before = []
if run_after is None:
run_after = []

def _execute_register_phase(obj):
import ramble.util.graph
Expand Down
2 changes: 1 addition & 1 deletion lib/ramble/ramble/mirror.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ def create(path, workspace):
return mirror_stats.stats()


def add(name, url, scope, args={}):
def add(name, url, scope):
"""Add a named mirror in the given scope"""
mirrors = ramble.config.get("mirrors", scope=scope)
if not mirrors:
Expand Down
5 changes: 4 additions & 1 deletion lib/ramble/ramble/modifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,12 @@ def format_doc(self, **kwargs):
results.write((" " * indent) + line + "\n")
return results.getvalue()

def modded_variables(self, app, extra_vars={}):
def modded_variables(self, app, extra_vars=None):
mods = {}

if extra_vars is None:
extra_vars = {}

if self._usage_mode not in self.variable_modifications:
return mods

Expand Down
4 changes: 2 additions & 2 deletions lib/ramble/ramble/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ def __init__(
self,
workspace,
filters,
output_formats=["text"],
output_formats=None,
upload=False,
print_results=False,
summary_only=False,
Expand All @@ -251,7 +251,7 @@ def __init__(

super().__init__(workspace, filters)
self.action_string = "Analyzing"
self.output_formats = output_formats
self.output_formats = ["text"] if output_formats is None else output_formats
self.require_inventory = True
self.upload_results = upload
self.print_results = print_results
Expand Down
4 changes: 2 additions & 2 deletions lib/ramble/ramble/software_environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def is_used(self):
"""
return self._used

def spec_str(self, all_packages: dict = {}, compiler=False):
def spec_str(self, all_packages=None, compiler=False):
"""Return a spec string for this software package

Args:
Expand Down Expand Up @@ -170,7 +170,7 @@ def __init__(
self.compiler_spec = compiler_spec
self._package_type = "Rendered"

def spec_str(self, all_packages: dict = {}, compiler=False):
def spec_str(self, all_packages=None, compiler=False):
"""Return a spec string for this software package

Args:
Expand Down
4 changes: 3 additions & 1 deletion lib/ramble/ramble/test/cmd/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
workspace = ramble.main.RambleCommand("workspace")


def _create_config(scope=None, data={}, section="repos"):
def _create_config(scope=None, data=None, section="repos"):
if data is None:
data = {}
scope = scope or ramble.config.default_modify_scope()
cfg_file = ramble.config.config.get_config_filename(scope, section)
with open(cfg_file, "w") as f:
Expand Down
4 changes: 3 additions & 1 deletion lib/ramble/ramble/test/stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,9 @@ def mock_stage_archive(tmp_build_stage_dir):
# <_hidden_fn> Optional hidden file (contains _hidden_contents)
# <_archive_fn> archive_url = file:///path/to/<_archive_fn>
#
def create_stage_archive(expected_file_list=[_include_readme]):
def create_stage_archive(expected_file_list=None):
if expected_file_list is None:
expected_file_list = [_include_readme]
tmpdir, test_stage_path = tmp_build_stage_dir
mkdirp(test_stage_path)

Expand Down
Loading
Loading