Skip to content

Commit 476d34f

Browse files
LanderOttoLanderOtto
committed
Handled the pickValue in the check_types function to avoid unnecessary warnings.
1 parent a2b22f2 commit 476d34f

File tree

2 files changed

+149
-15
lines changed

2 files changed

+149
-15
lines changed

‎cwltool/checker.py‎

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ def check_types(
2424
srctype: SinkType,
2525
sinktype: SinkType,
2626
linkMerge: str | None,
27+
pickValue: str | None,
2728
valueFrom: str | None,
2829
) -> Literal["pass"] | Literal["warning"] | Literal["exception"]:
2930
"""
@@ -33,23 +34,49 @@ def check_types(
3334
"""
3435
if valueFrom is not None:
3536
return "pass"
37+
if pickValue is not None:
38+
if (
39+
isinstance((_srctype := _get_type(srctype)), MutableMapping)
40+
and _srctype["type"] == "array"
41+
):
42+
match pickValue:
43+
case "all_non_null":
44+
_srctype = {"type": "array", "items": _srctype["items"]}
45+
if (
46+
isinstance(_srctype["items"], MutableSequence)
47+
and "null" in _srctype["items"]
48+
):
49+
_srctype["items"].remove("null")
50+
case "first_non_null" | "the_only_non_null":
51+
if (
52+
isinstance(_srctype["items"], MutableSequence)
53+
and "null" in _srctype["items"]
54+
):
55+
_srctype = [elem for elem in _srctype["items"] if elem != "null"]
56+
case _:
57+
raise WorkflowException(f"Unrecognized pickValue enum {pickValue!r}")
58+
_sinktype = _get_type(sinktype)
59+
else:
60+
_srctype = srctype
61+
_sinktype = sinktype
3662
match linkMerge:
3763
case None:
38-
if can_assign_src_to_sink(srctype, sinktype, strict=True):
64+
if can_assign_src_to_sink(_srctype, _sinktype, strict=True):
3965
return "pass"
40-
if can_assign_src_to_sink(srctype, sinktype, strict=False):
66+
if can_assign_src_to_sink(_srctype, _sinktype, strict=False):
4167
return "warning"
4268
return "exception"
4369
case "merge_nested":
4470
return check_types(
45-
{"items": _get_type(srctype), "type": "array"},
71+
{"items": _get_type(_srctype), "type": "array"},
4672
_get_type(sinktype),
4773
None,
4874
None,
75+
None,
4976
)
5077
case "merge_flattened":
5178
return check_types(
52-
merge_flatten_type(_get_type(srctype)), _get_type(sinktype), None, None
79+
merge_flatten_type(_get_type(_srctype)), _get_type(sinktype), None, None, None
5380
)
5481
case _:
5582
raise WorkflowException(f"Unrecognized linkMerge enum {linkMerge!r}")
@@ -370,7 +397,15 @@ def _check_all_types(
370397
srcs_of_sink: list[CWLObjectType] = []
371398
for parm_id in cast(MutableSequence[str], sink[sourceField]):
372399
srcs_of_sink += [src_dict[parm_id]]
373-
if is_conditional_step(param_to_step, parm_id) and pickValue is None:
400+
sink_type = cast(
401+
Union[str, list[str], list[CWLObjectType], CWLObjectType], sink["type"]
402+
)
403+
if (
404+
is_conditional_step(param_to_step, parm_id)
405+
and "null" != sink_type
406+
and "null" not in sink_type
407+
and pickValue is None
408+
):
374409
validation["warning"].append(
375410
_SrcSink(
376411
src_dict[parm_id],
@@ -432,7 +467,7 @@ def _check_all_types(
432467
}
433468

434469
for src in srcs_of_sink:
435-
check_result = check_types(src, sink, linkMerge, valueFrom)
470+
check_result = check_types(src, sink, linkMerge, pickValue, valueFrom)
436471
if check_result == "warning":
437472
validation["warning"].append(
438473
_SrcSink(src, sink, linkMerge, message=extra_message)

‎tests/test_examples.py‎

Lines changed: 108 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -681,106 +681,119 @@ def test_compare_types_strict(
681681

682682

683683
typechecks = [
684-
(["string", "int"], ["string", "int", "null"], None, None, "pass"),
685-
(["string", "int"], ["string", "null"], None, None, "warning"),
686-
(["File", "int"], ["string", "null"], None, None, "exception"),
684+
(["string", "int"], ["string", "int", "null"], None, None, None, "pass"),
685+
(["string", "int"], ["string", "null"], None, None, None, "warning"),
686+
(["File", "int"], ["string", "null"], None, None, None, "exception"),
687687
(
688688
{"items": ["string", "int"], "type": "array"},
689689
{"items": ["string", "int", "null"], "type": "array"},
690690
None,
691691
None,
692+
None,
692693
"pass",
693694
),
694695
(
695696
{"items": ["string", "int"], "type": "array"},
696697
{"items": ["string", "null"], "type": "array"},
697698
None,
698699
None,
700+
None,
699701
"warning",
700702
),
701703
(
702704
{"items": ["File", "int"], "type": "array"},
703705
{"items": ["string", "null"], "type": "array"},
704706
None,
705707
None,
708+
None,
706709
"exception",
707710
),
708711
# check linkMerge when sinktype is not an array
709-
(["string", "int"], ["string", "int", "null"], "merge_nested", None, "exception"),
712+
(["string", "int"], ["string", "int", "null"], "merge_nested", None, None, "exception"),
710713
# check linkMerge: merge_nested
711714
(
712715
["string", "int"],
713716
{"items": ["string", "int", "null"], "type": "array"},
714717
"merge_nested",
715718
None,
719+
None,
716720
"pass",
717721
),
718722
(
719723
["string", "int"],
720724
{"items": ["string", "null"], "type": "array"},
721725
"merge_nested",
722726
None,
727+
None,
723728
"warning",
724729
),
725730
(
726731
["File", "int"],
727732
{"items": ["string", "null"], "type": "array"},
728733
"merge_nested",
729734
None,
735+
None,
730736
"exception",
731737
),
732738
# check linkMerge: merge_nested and sinktype is "Any"
733-
(["string", "int"], "Any", "merge_nested", None, "pass"),
739+
(["string", "int"], "Any", "merge_nested", None, None, "pass"),
734740
# check linkMerge: merge_flattened
735741
(
736742
["string", "int"],
737743
{"items": ["string", "int", "null"], "type": "array"},
738744
"merge_flattened",
739745
None,
746+
None,
740747
"pass",
741748
),
742749
(
743750
["string", "int"],
744751
{"items": ["string", "null"], "type": "array"},
745752
"merge_flattened",
746753
None,
754+
None,
747755
"warning",
748756
),
749757
(
750758
["File", "int"],
751759
{"items": ["string", "null"], "type": "array"},
752760
"merge_flattened",
753761
None,
762+
None,
754763
"exception",
755764
),
756765
(
757766
{"items": ["string", "int"], "type": "array"},
758767
{"items": ["string", "int", "null"], "type": "array"},
759768
"merge_flattened",
760769
None,
770+
None,
761771
"pass",
762772
),
763773
(
764774
{"items": ["string", "int"], "type": "array"},
765775
{"items": ["string", "null"], "type": "array"},
766776
"merge_flattened",
767777
None,
778+
None,
768779
"warning",
769780
),
770781
(
771782
{"items": ["File", "int"], "type": "array"},
772783
{"items": ["string", "null"], "type": "array"},
773784
"merge_flattened",
774785
None,
786+
None,
775787
"exception",
776788
),
777789
# check linkMerge: merge_flattened and sinktype is "Any"
778-
(["string", "int"], "Any", "merge_flattened", None, "pass"),
790+
(["string", "int"], "Any", "merge_flattened", None, None, "pass"),
779791
(
780792
{"items": ["string", "int"], "type": "array"},
781793
"Any",
782794
"merge_flattened",
783795
None,
796+
None,
784797
"pass",
785798
),
786799
# check linkMerge: merge_flattened when srctype is a list
@@ -789,25 +802,111 @@ def test_compare_types_strict(
789802
{"items": "string", "type": "array"},
790803
"merge_flattened",
791804
None,
805+
None,
806+
"pass",
807+
),
808+
# check pickValue: all_non_null
809+
(
810+
{"items": ["null", "string"], "type": "array"},
811+
{"items": "string", "type": "array"},
812+
None,
813+
"all_non_null",
814+
None,
815+
"pass",
816+
),
817+
(
818+
{"items": ["null", "string"], "type": "array"},
819+
{"items": "string", "type": "array"},
820+
None,
821+
None,
822+
None,
823+
"warning",
824+
),
825+
(
826+
{"items": ["null", "string"], "type": "array"},
827+
"string",
828+
None,
829+
"all_non_null",
830+
None,
831+
"exception",
832+
),
833+
# check pickValue: first_non_null
834+
(
835+
{"items": ["null", "string"], "type": "array"},
836+
"string",
837+
None,
838+
"first_non_null",
839+
None,
840+
"pass",
841+
),
842+
(
843+
{"items": ["null", "string"], "type": "array"},
844+
"int",
845+
None,
846+
"first_non_null",
847+
None,
848+
"exception",
849+
),
850+
# check pickValue: the_only_non_null
851+
(
852+
{"items": ["null", "int"], "type": "array"},
853+
"int",
854+
None,
855+
"the_only_non_null",
856+
None,
857+
"pass",
858+
),
859+
# check pickValue: all_non_null and linkMerge: merge_nested
860+
(
861+
[
862+
{"items": ["null", "string"], "type": "array"},
863+
{"items": ["null", "File"], "type": "array"},
864+
],
865+
{"items": {"items": ["string", "File"], "type": "array"}, "type": "array"},
866+
"merge_nested",
867+
"all_non_null",
868+
None,
869+
"pass",
870+
),
871+
# check pickValue: all_non_null and linkMerge: merge_flattened
872+
(
873+
[
874+
{"items": ["null", "string"], "type": "array"},
875+
{"items": ["null", "Directory"], "type": "array"},
876+
],
877+
{"items": ["string", "Directory"], "type": "array"},
878+
"merge_flattened",
879+
"all_non_null",
880+
None,
792881
"pass",
793882
),
794883
# check valueFrom
795884
(
796885
{"items": ["File", "int"], "type": "array"},
797886
{"items": ["string", "null"], "type": "array"},
798887
"merge_flattened",
888+
None,
799889
"special value",
800890
"pass",
801891
),
802892
]
803893

804894

805-
@pytest.mark.parametrize("src_type,sink_type,link_merge,value_from,expected_type", typechecks)
895+
@pytest.mark.parametrize(
896+
"src_type,sink_type,link_merge,pick_value,value_from,expected_type", typechecks
897+
)
806898
def test_typechecking(
807-
src_type: Any, sink_type: Any, link_merge: str, value_from: Any, expected_type: str
899+
src_type: Any,
900+
sink_type: Any,
901+
link_merge: str,
902+
pick_value: Any,
903+
value_from: Any,
904+
expected_type: str,
808905
) -> None:
809906
assert (
810-
cwltool.checker.check_types(src_type, sink_type, linkMerge=link_merge, valueFrom=value_from)
907+
cwltool.checker.check_types(
908+
src_type, sink_type, linkMerge=link_merge, pickValue=pick_value, valueFrom=value_from
909+
)
811910
== expected_type
812911
)
813912

0 commit comments

Comments
 (0)