Skip to content

Fix class detection for namespaced classes (Py)#897

Merged
mergify[bot] merged 1 commit intomainfrom
fix-prefixed-classes
Dec 4, 2024
Merged

Fix class detection for namespaced classes (Py)#897
mergify[bot] merged 1 commit intomainfrom
fix-prefixed-classes

Conversation

@GlassOfWhiskey
Copy link
Copy Markdown
Contributor

This commit asjusts the Python generated parser to correctly deal with namespaced classes (e.g., those coming from cwltool extensions).

@GlassOfWhiskey GlassOfWhiskey requested review from mr-c and tetron December 4, 2024 15:30
@GlassOfWhiskey GlassOfWhiskey force-pushed the fix-prefixed-classes branch 10 times, most recently from b1a38c5 to db52266 Compare December 4, 2024 16:48
This commit asjusts the Python generated parser to correctly deal with
namespaced classes (e.g., those coming from cwltool extensions).
@mr-c
Copy link
Copy Markdown
Member

mr-c commented Dec 4, 2024

Thank you @GlassOfWhiskey !

I regenerated the cwl-utils parsers and ran the tests, I get one error:

_________________________________________ test_static_checker_fail[v1_0] _________________________________________

cwlVersion = 'v1_0'

    @pytest.mark.parametrize("cwlVersion", ["v1_0", "v1_1", "v1_2"])
    def test_static_checker_fail(cwlVersion: str) -> None:
        """Confirm that static type checker raises expected exception."""
        if cwlVersion == "v1_0":
            uri = Path(get_data("testdata/checker_wf/broken-wf.cwl")).resolve().as_uri()
            cwl_obj = load_document_by_uri(uri)
            with pytest.raises(
                ValidationException,
                match="\nSource .* of type .* is incompatible\n.*with sink .* of type .*",
            ):
                cwl_utils.parser.utils.static_checker(cwl_obj)
    
            uri = Path(get_data("testdata/checker_wf/broken-wf2.cwl")).resolve().as_uri()
            cwl_obj = load_document_by_uri(uri)
            with pytest.raises(
                ValidationException, match="param .* not found in class: CommandLineTool"
            ):
>               cwl_utils.parser.utils.static_checker(cwl_obj)

tests/test_parser_utils.py:42: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
cwl_utils/parser/utils.py:193: in static_checker
    {
cwl_utils/parser/utils.py:194: in <dictcomp>
    s.id: type_for_step_output(step, s.id, workflow.cwlVersion)
cwl_utils/parser/utils.py:410: in type_for_step_output
    return cwl_v1_0_utils.type_for_step_output(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

step = <cwl_utils.parser.cwl_v1_0.WorkflowStep object at 0x7f2025d3fc10>
sourcename = 'file:///home/michael/cwl-utils/testdata/checker_wf/broken-wf2.cwl#echo_w/other'

    def type_for_step_output(
        step: cwl.WorkflowStep,
        sourcename: str,
    ) -> Any:
        """Determine the type for the given step output."""
        step_run = cwl_utils.parser.utils.load_step(step)
        cwl_utils.parser.utils.convert_stdstreams_to_files(step_run)
        if step_run and step_run.outputs:
            for step_output in step_run.outputs:
                if (
                    step_output.id.split("#")[-1].split("/")[-1]
                    == sourcename.split("#")[-1].split("/")[-1]
                ):
                    output_type = step_output.type_
                    if step.scatter is not None:
                        if step.scatterMethod == "nested_crossproduct":
                            for _ in range(len(aslist(step.scatter))):
                                output_type = cwl.ArraySchema(
                                    items=output_type, type_="array"
                                )
                        else:
                            output_type = cwl.ArraySchema(items=output_type, type_="array")
                    return output_type
>       raise ValidationException(
            "param {} not found in {}.".format(
                sourcename,
                yaml_dumps(cwl.save(step_run)),
            )
        )
E       schema_salad.exceptions.ValidationException: param file:///home/michael/cwl-utils/testdata/checker_wf/broken-wf2.cwl#echo_w/other not found in id: file:///home/michael/cwl-utils/testdata/checker_wf/echo.cwl
E       class: CommandLineTool
E       inputs:
E       - id: echo_in
E         inputBinding: {}
E         type:
E         - string
E         - items: string
E           type: array
E       outputs:
E       - id: txt
E         outputBinding:
E           glob: out.txt
E         type: File
E       cwlVersion: v1.0
E       baseCommand: echo
E       stdout: out.txt
E       .

cwl_utils/parser/cwl_v1_0_utils.py:417: ValidationException

During handling of the above exception, another exception occurred:

cwlVersion = 'v1_0'

    @pytest.mark.parametrize("cwlVersion", ["v1_0", "v1_1", "v1_2"])
    def test_static_checker_fail(cwlVersion: str) -> None:
        """Confirm that static type checker raises expected exception."""
        if cwlVersion == "v1_0":
            uri = Path(get_data("testdata/checker_wf/broken-wf.cwl")).resolve().as_uri()
            cwl_obj = load_document_by_uri(uri)
            with pytest.raises(
                ValidationException,
                match="\nSource .* of type .* is incompatible\n.*with sink .* of type .*",
            ):
                cwl_utils.parser.utils.static_checker(cwl_obj)
    
            uri = Path(get_data("testdata/checker_wf/broken-wf2.cwl")).resolve().as_uri()
            cwl_obj = load_document_by_uri(uri)
>           with pytest.raises(
                ValidationException, match="param .* not found in class: CommandLineTool"
            ):
E           AssertionError: Regex pattern did not match.
E            Regex: 'param .* not found in class: CommandLineTool'
E            Input: 'param file:///home/michael/cwl-utils/testdata/checker_wf/broken-wf2.cwl#echo_w/other not found in id: file:///home/michael/cwl-utils/testdata/checker_wf/echo.cwl\nclass: CommandLineTool\ninputs:\n- id: echo_in\n  inputBinding: {}\n  type:\n  - string\n  - items: string\n    type: array\noutputs:\n- id: txt\n  outputBinding:\n    glob: out.txt\n  type: File\ncwlVersion: v1.0\nbaseCommand: echo\nstdout: out.txt\n.'

tests/test_parser_utils.py:39: AssertionError

Copy link
Copy Markdown
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

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

That error looks harmless, we just need to update the other test to match the re-ordered output.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.13%. Comparing base (f3518e2) to head (daf7967).
Report is 26 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #897      +/-   ##
==========================================
- Coverage   83.14%   83.13%   -0.01%     
==========================================
  Files          22       22              
  Lines        4900     4899       -1     
  Branches     1178     1177       -1     
==========================================
- Hits         4074     4073       -1     
  Misses        548      548              
  Partials      278      278              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mergify mergify Bot merged commit 54a657b into main Dec 4, 2024
@mergify mergify Bot deleted the fix-prefixed-classes branch December 4, 2024 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants