Skip to content

Commit e39ea8a

Browse files
authored
Minor readability refactor (PolusAI#342)
1 parent 802aec1 commit e39ea8a

File tree

3 files changed

+68
-63
lines changed

3 files changed

+68
-63
lines changed

src/sophios/api/pythonapi.py

Lines changed: 62 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -405,34 +405,39 @@ def _validate(self) -> None:
405405
def _yml(self) -> dict:
406406
in_dict: dict[str, Any] = {} # NOTE: input values can be arbitrary JSON; not just strings!
407407
for inp in self.inputs:
408-
if inp.value is not None:
409-
if isinstance(inp.value, Path):
410-
# Special case for Path since it does not inherit from YAMLObject
411-
in_dict[inp.name] = str(inp.value)
412-
elif isinstance(inp.value, dict) and isinstance(inp.value.get('wic_alias', {}), Path):
413-
# Special case for Path since it does not inherit from YAMLObject
414-
in_dict[inp.name] = {'wic_alias': str(inp.value['wic_alias'])}
415-
elif isinstance(inp.value, dict) and isinstance(inp.value.get('wic_inline_input', {}), Path):
416-
# Special case for Path since it does not inherit from YAMLObject
417-
in_dict[inp.name] = {'wic_inline_input': str(inp.value['wic_inline_input'])}
418-
elif isinstance(inp.value, dict) and isinstance(inp.value.get('wic_inline_input', {}), str):
419-
# Special case for inline str since it does not inherit from YAMLObject
420-
in_dict[inp.name] = {'wic_inline_input': inp.value.get('wic_inline_input')}
421-
elif isinstance(inp.value, str):
422-
in_dict[inp.name] = inp.value # Obviously strings are serializable
423-
elif isinstance(inp.value, yaml.YAMLObject):
424-
# Serialization and deserialization logic should always be
425-
# encapsulated within each object. For the pyyaml library,
426-
# each object should inherit from pyyaml.YAMLObject.
427-
# See https://pyyaml.org/wiki/PyYAMLDocumentation
428-
# Section "Constructors, representers, resolvers"
429-
# class Monster(yaml.YAMLObject): ...
430-
in_dict[inp.name] = inp.value
431-
else:
432-
logger.warning(f'Warning! input name {inp.name} input value {inp.value}')
433-
logger.warning('is not an instance of YAMLObject. The default str() serialization')
434-
logger.warning('logic often gives bad results. Please explicitly inherit from YAMLObject.')
435-
in_dict[inp.name] = inp.value
408+
if inp.value:
409+
match inp.value:
410+
case Path():
411+
# Special case for Path since it does not inherit from YAMLObject
412+
in_dict[inp.name] = str(inp.value)
413+
case {'wic_alias': wic_alias, **rest_of_dict}:
414+
match wic_alias:
415+
case Path():
416+
# Special case for Path since it does not inherit from YAMLObject
417+
in_dict[inp.name] = {'wic_alias': str(inp.value['wic_alias'])}
418+
case {'wic_inline_input': wic_inline_input, **rest_of_dict}:
419+
match wic_inline_input:
420+
case Path():
421+
# Special case for Path since it does not inherit from YAMLObject
422+
in_dict[inp.name] = {'wic_inline_input': str(inp.value['wic_inline_input'])}
423+
case str():
424+
# Special case for inline str since it does not inherit from YAMLObject
425+
in_dict[inp.name] = {'wic_inline_input': inp.value.get('wic_inline_input')}
426+
case str():
427+
in_dict[inp.name] = inp.value # Obviously strings are serializable
428+
case yaml.YAMLObject():
429+
# Serialization and deserialization logic should always be
430+
# encapsulated within each object. For the pyyaml library,
431+
# each object should inherit from pyyaml.YAMLObject.
432+
# See https://pyyaml.org/wiki/PyYAMLDocumentation
433+
# Section "Constructors, representers, resolvers"
434+
# class Monster(yaml.YAMLObject): ...
435+
in_dict[inp.name] = inp.value
436+
case _:
437+
logger.warning(f'Warning! input name {inp.name} input value {inp.value}')
438+
logger.warning('is not an instance of YAMLObject. The default str() serialization')
439+
logger.warning('logic often gives bad results. Please explicitly inherit from YAMLObject.')
440+
in_dict[inp.name] = inp.value
436441

437442
out_list: list = [] # The out: tag is a list, not a dict
438443
out_list = [{out.name: out.value} for out in self.outputs if out.value]
@@ -523,11 +528,13 @@ def _validate(self) -> None:
523528
# subworkflows will NOT have all required inputs.
524529
for s in self.steps:
525530
try:
526-
if isinstance(s, Step):
527-
s._validate() # pylint: disable=W0212
528-
if isinstance(s, Workflow):
529-
# recursively validate subworkflows ?
530-
s._validate() # pylint: disable=W0212
531+
match s:
532+
case Step():
533+
s._validate()
534+
case Workflow():
535+
s._validate()
536+
case _:
537+
pass
531538
except BaseException as exc:
532539
raise InvalidStepError(
533540
f"{s.process_name} is missing required inputs"
@@ -558,21 +565,23 @@ def yaml(self) -> dict[str, Any]:
558565
# TODO: outputs?
559566
steps = []
560567
for s in self.steps:
561-
if isinstance(s, Step):
562-
steps.append(s._yml)
563-
elif isinstance(s, Workflow):
564-
ins = {
565-
inp.name: inp.value
566-
for inp in s.inputs
567-
if inp.value is not None # Subworkflow args are not required
568-
}
569-
parentargs: dict[str, Any] = {"in": ins} if ins else {}
570-
# See the second to last line of ast.read_ast_from_disk()
571-
d = {'id': self.process_name + '.wic',
572-
'subtree': s.yaml, # recursively call .yaml (i.e. on s, not self)
573-
'parentargs': parentargs}
574-
steps.append(d)
575-
# else: ...
568+
match s:
569+
case Step():
570+
steps.append(s._yml)
571+
case Workflow() as sw:
572+
ins = {
573+
inp.name: inp.value
574+
for inp in sw.inputs
575+
if inp.value is not None # Subworkflow args are not required
576+
}
577+
parentargs: dict[str, Any] = {"in": ins} if ins else {}
578+
# See the second to last line of ast.read_ast_from_disk()
579+
d = {'id': self.process_name + '.wic',
580+
'subtree': sw.yaml, # recursively call .yaml (i.e. on s, not self)
581+
'parentargs': parentargs}
582+
steps.append(d)
583+
case _:
584+
pass
576585
yaml_contents = {"inputs": inputs, "steps": steps} if inputs else {"steps": steps}
577586
return yaml_contents
578587

@@ -700,10 +709,11 @@ def flatten_steps(self) -> list[Step]:
700709
"""
701710
steps = []
702711
for step in self.steps:
703-
if isinstance(step, Step):
704-
steps.append(step)
705-
if isinstance(step, Workflow):
706-
steps += step.flatten_steps()
712+
match step:
713+
case Step():
714+
steps.append(step)
715+
case Workflow():
716+
steps += step.flatten_steps()
707717
return steps
708718

709719
# NOTE: Cannot return list[Workflow] because Workflow is not yet defined.
@@ -773,8 +783,6 @@ def get_cwl_workflow(self, args_dict: Dict[str, str] = {}) -> Json:
773783
Returns:
774784
Json: Contains the compiled CWL and yaml inputs to the workflow.
775785
"""
776-
user_args = self._convert_args_dict_to_args_list(args_dict)
777-
args = get_args(self.process_name, user_args)
778786
compiler_info = self.compile(args_dict=args_dict, write_to_disk=False)
779787
rose_tree = compiler_info.rose
780788

src/sophios/api/utils/ict/ict_spec/tools/cwl_ict.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,11 @@ def clt_dict(ict_: "ICT", network_access: bool) -> dict:
5353

5454
def remove_none(d: Union[dict, str]) -> Union[dict, str]:
5555
"""Recursively remove keys with None values."""
56-
if isinstance(d, dict):
57-
return {k: remove_none(v) for k, v in d.items() if v is not None}
58-
elif isinstance(d, str):
59-
return d # Return the string unchanged
60-
else:
61-
return d # Return other types of values unchanged
56+
match d:
57+
case dict():
58+
return {k: remove_none(v) for k, v in d.items() if v is not None}
59+
case str():
60+
return d
6261

6362

6463
def input_output_dict(ict_: "ICT") -> Union[dict, str]:

src/sophios/main.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ def main() -> None:
3939
args.validate_plugins,
4040
not args.no_skip_dollar_schemas,
4141
args.quiet)
42-
# This takes ~1 second but it is not really necessary.
43-
# utils_graphs.make_plugins_dag(tools_cwl, args.graph_dark_theme)
4442
# pass around config object instead of reading from the disk!
4543
yml_paths = plugins.get_yml_paths(global_config)
4644

@@ -55,7 +53,7 @@ def main() -> None:
5553

5654
# Generating yml schemas every time takes ~20 seconds and guarantees the
5755
# subworkflow schemas are always up to date. However, since it compiles all
58-
# yml files, if there are any errors in any of the yml files, the user may
56+
# wic files, if there are any errors in any of the wic files, the user may
5957
# be confused by an error message when the --yaml file is correct.
6058
# For now, require the user to update the schemas manually. In the future,
6159
# we may use a filewatcher.

0 commit comments

Comments
 (0)