Skip to content

Commit bb1b705

Browse files
jmchiltonclaude
andcommitted
Use gxformat2 0.25 lint interface; drop duplicated best-practices
Bump gxformat2>=0.25.0; switch workflow_lint to lint_*_path and lint_best_practices_{ga,format2}; delete the duplicated planemo _lint_best_practices body that was migrated upstream. Wire the new lint_pydantic_validation as a schema_validation lint step. Coerce gxformat2 Linter classes to their names in WorkflowLintContext so galaxy's LintMessage doesn't render class reprs. Update test assertions for the new message text (drops "with ID", uses native step ids directly). Clean up three legacy test fixtures that strict pydantic validation now rejects: null input key in wf14-unlinted-best-practices.yml and format2-style top-level inputs/ outputs on two native basic_native fixtures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 836bcae commit bb1b705

6 files changed

Lines changed: 49 additions & 131 deletions

File tree

planemo/workflow_lint.py

Lines changed: 36 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import inspect
2-
import json
32
import os
43
import re
5-
from collections import OrderedDict
64
from typing import (
75
Any,
86
Dict,
@@ -13,7 +11,6 @@
1311
Tuple,
1412
TYPE_CHECKING,
1513
)
16-
from urllib.parse import urlparse
1714

1815
import requests
1916
import yaml
@@ -24,8 +21,11 @@
2421
from galaxy.tool_util.parser.yaml import __to_test_assert_list
2522
from galaxy.tool_util.verify import asserts
2623
from gxformat2.lint import (
27-
lint_format2,
28-
lint_ga,
24+
lint_best_practices_format2,
25+
lint_best_practices_ga,
26+
lint_format2_path,
27+
lint_ga_path,
28+
lint_pydantic_validation,
2929
)
3030
from gxformat2.yaml import ordered_load
3131

@@ -59,6 +59,17 @@ class WorkflowLintContext(LintContext):
5959
# from click arguments.
6060
training_topic = None
6161

62+
def warn(self, message, linter=None, *args, **kwargs):
63+
# gxformat2 lint rules pass Linter subclasses; galaxy LintMessage expects a name string.
64+
if isinstance(linter, type):
65+
linter = linter.__name__
66+
super().warn(message, linter, *args, **kwargs)
67+
68+
def error(self, message, linter=None, *args, **kwargs):
69+
if isinstance(linter, type):
70+
linter = linter.__name__
71+
super().error(message, linter, *args, **kwargs)
72+
6273

6374
def build_wf_lint_args(ctx: "PlanemoCliContext", **kwds) -> Dict[str, Any]:
6475
lint_args = build_lint_args(ctx, **kwds)
@@ -158,15 +169,8 @@ def _lint_workflow_artifacts_on_path(lint_context: WorkflowLintContext, path: st
158169
)
159170

160171
elif looks_like_a_workflow(potential_workflow_artifact_path):
161-
162-
def structure(path, lint_context):
163-
with open(path) as f:
164-
workflow_dict = ordered_load(f)
165-
workflow_class = workflow_dict.get("class")
166-
lint_func = lint_format2 if workflow_class == "GalaxyWorkflow" else lint_ga
167-
lint_func(lint_context, workflow_dict, path=path)
168-
169-
lint_context.lint("lint_structure", structure, potential_workflow_artifact_path)
172+
lint_context.lint("lint_structure", _lint_structure, potential_workflow_artifact_path)
173+
lint_context.lint("lint_schema_validation", _lint_schema_validation, potential_workflow_artifact_path)
170174
if lint_args["iwc_grade"]:
171175
lint_context.lint("lint_release", _lint_release, potential_workflow_artifact_path)
172176
lint_context.lint("lint_best_practices", _lint_best_practices, potential_workflow_artifact_path)
@@ -200,91 +204,31 @@ def _lint_tsts(path: str, lint_context: WorkflowLintContext) -> None:
200204
lint_context.valid(f"Tests appear structurally correct for {runnable.path}")
201205

202206

203-
def _lint_best_practices(path: str, lint_context: WorkflowLintContext) -> None: # noqa: C901
204-
"""
205-
This function duplicates the checks made by Galaxy's best practices panel:
206-
https://github.com/galaxyproject/galaxy/blob/5396bb15fe8cfcf2e89d46c1d061c49b60e2f0b1/client/src/components/Workflow/Editor/Lint.vue
207-
"""
208-
209-
def check_json_for_untyped_params(j):
210-
values = j.values() if isinstance(j, dict) else j
211-
for value in values:
212-
if type(value) in [list, dict, OrderedDict]:
213-
if check_json_for_untyped_params(value):
214-
return True
215-
elif isinstance(value, str):
216-
if re.match(r"\$\{.+?\}", value):
217-
return True
218-
return False
207+
def _lint_structure(path: str, lint_context: WorkflowLintContext) -> None:
208+
workflow_dict = _load_workflow_dict(path)
209+
if workflow_dict.get("class") == "GalaxyWorkflow":
210+
lint_format2_path(lint_context, path)
211+
else:
212+
lint_ga_path(lint_context, path)
219213

220-
with open(path) as f:
221-
workflow_dict = ordered_load(f)
222214

223-
steps = workflow_dict.get("steps", {})
215+
def _lint_schema_validation(path: str, lint_context: WorkflowLintContext) -> None:
216+
workflow_dict = _load_workflow_dict(path)
217+
is_format2 = workflow_dict.get("class") == "GalaxyWorkflow"
218+
lint_pydantic_validation(lint_context, workflow_dict, format2=is_format2)
224219

225-
# annotation
226-
if not workflow_dict.get("annotation"):
227-
lint_context.warn("Workflow is not annotated.")
228220

229-
# creator
230-
creators = workflow_dict.get("creator", [])
231-
if not len(creators) > 0:
232-
lint_context.warn("Workflow does not specify a creator.")
221+
def _lint_best_practices(path: str, lint_context: WorkflowLintContext) -> None:
222+
workflow_dict = _load_workflow_dict(path)
223+
if workflow_dict.get("class") == "GalaxyWorkflow":
224+
lint_best_practices_format2(lint_context, workflow_dict)
233225
else:
234-
if not isinstance(creators, list):
235-
# Don't know if this can happen, if we implement schema validation on the Galaxy side
236-
# this won't be needed.
237-
creators = [creators]
238-
for creator in creators:
239-
if creator.get("class", "").lower() == "person" and "identifier" in creator:
240-
identifier = creator["identifier"]
241-
parsed_url = urlparse(identifier)
242-
if not parsed_url.scheme:
243-
lint_context.warn(
244-
f'Creator identifier "{identifier}" should be a fully qualified URI, for example "https://orcid.org/0000-0002-1825-0097".'
245-
)
246-
247-
# license
248-
if not workflow_dict.get("license"):
249-
lint_context.warn("Workflow does not specify a license.")
250-
251-
# checks on individual steps
252-
for step in steps.values():
253-
# disconnected inputs
254-
if step.get("type") not in ["data_collection_input", "parameter_input"]:
255-
for input in step.get("inputs", []):
256-
if input.get("name") not in step.get("input_connections"): # TODO: check optional
257-
lint_context.warn(
258-
f"Input {input.get('name')} of workflow step {step.get('annotation') or step.get('id')} is disconnected."
259-
)
260-
261-
# missing metadata
262-
if not step.get("annotation"):
263-
lint_context.warn(f"Workflow step with ID {step.get('id')} has no annotation.")
264-
if not step.get("label"):
265-
lint_context.warn(f"Workflow step with ID {step.get('id')} has no label.")
266-
267-
# untyped parameters
268-
if workflow_dict.get("class") == "GalaxyWorkflow":
269-
tool_state = step.get("tool_state", {})
270-
pjas = step.get("out", {})
271-
else:
272-
raw_tool_state = step.get("tool_state", {})
273-
if isinstance(raw_tool_state, str):
274-
tool_state = json.loads(raw_tool_state)
275-
else:
276-
tool_state = raw_tool_state
277-
pjas = step.get("post_job_actions", {})
226+
lint_best_practices_ga(lint_context, workflow_dict)
278227

279-
if check_json_for_untyped_params(tool_state):
280-
lint_context.warn(f"Workflow step with ID {step.get('id')} specifies an untyped parameter as an input.")
281228

282-
if check_json_for_untyped_params(pjas):
283-
lint_context.warn(
284-
f"Workflow step with ID {step.get('id')} specifies an untyped parameter in the post-job actions."
285-
)
286-
287-
# unlabeled outputs are checked by gxformat2, no need to check here
229+
def _load_workflow_dict(path: str) -> Dict[str, Any]:
230+
with open(path) as f:
231+
return ordered_load(f)
288232

289233

290234
def _lint_case(path: str, test_case: TestCase, lint_context: WorkflowLintContext) -> bool:

requirements.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ galaxy-job-config-init>=0.1.4
88
galaxy-tool-util[edam,extended-assertions]>=25.1,<26.0
99
galaxy-util[template]>=24.1,<26.0
1010
glob2
11-
gxformat2>=0.14.0
11+
gxformat2>=0.25.0
1212
h5py
1313
jinja2
1414
lxml

tests/data/wf14-unlinted-best-practices.yml

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,8 @@ class: GalaxyWorkflow
22
label: bp (imported from uploaded file)
33
uuid: 66708ffe-c08c-4647-a7e9-fc7f731206a1
44
inputs:
5-
null:
5+
input:
66
optional: false
7-
position:
8-
bottom: 730.3166656494141
9-
height: 82.55000305175781
10-
left: 1155
11-
right: 1355
12-
top: 647.7666625976562
13-
width: 200
14-
x: 1155
15-
y: 647.7666625976562
167
type: data
178
outputs:
189
_anonymous_output_1:

tests/data/wf_repos/basic_native_ok/basic_native.yml

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,3 @@
1-
inputs:
2-
the_input:
3-
type: File
4-
doc: input doc
5-
id: the_input
6-
outputs:
7-
the_output:
8-
outputSource: cat/out_file1
91
steps:
102
'0':
113
type: data_input

tests/data/wf_repos/from_format2/0_basic_native/0_basic_native.yml

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,3 @@
1-
inputs:
2-
the_input:
3-
type: File
4-
doc: input doc
5-
id: the_input
6-
outputs:
7-
the_output:
8-
outputSource: cat/out_file1
91
steps:
102
'0':
113
type: data_input

tests/test_cmd_workflow_lint.py

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,10 @@ def test_best_practices_linting_gx(self):
142142
"Workflow is not annotated.",
143143
"Workflow does not specify a creator.",
144144
"Workflow does not specify a license.",
145-
"Workflow step with ID None has no annotation.",
146-
"Workflow step with ID None has no label.",
145+
"Workflow step input has no annotation.",
147146
"Workflow missing test cases.",
148-
"Workflow step with ID None specifies an untyped parameter as an input.",
149-
"Workflow step with ID None specifies an untyped parameter in the post-job actions.",
147+
"Workflow step input specifies an untyped parameter as an input.",
148+
"Workflow step input specifies an untyped parameter in the post-job actions.",
150149
]
151150

152151
for warning in warnings:
@@ -161,11 +160,11 @@ def test_best_practices_linting_ga(self):
161160
"Workflow is not annotated.",
162161
"Workflow does not specify a creator.",
163162
"Workflow does not specify a license.",
164-
"Workflow step with ID 0 has no annotation.",
165-
"Workflow step with ID 0 has no label.",
163+
"Workflow step 0 has no annotation.",
164+
"Workflow step 0 has no label.",
166165
"Workflow missing test cases.",
167-
"Workflow step with ID 1 specifies an untyped parameter as an input.",
168-
"Workflow step with ID 1 specifies an untyped parameter in the post-job actions.",
166+
"Workflow step 1 specifies an untyped parameter as an input.",
167+
"Workflow step 1 specifies an untyped parameter in the post-job actions.",
169168
]
170169

171170
for warning in warnings:
@@ -180,11 +179,11 @@ def test_best_practices_linting_ga_dict_tool_state(self):
180179
"Workflow is not annotated.",
181180
"Workflow does not specify a creator.",
182181
"Workflow does not specify a license.",
183-
"Workflow step with ID 0 has no annotation.",
184-
"Workflow step with ID 0 has no label.",
182+
"Workflow step 0 has no annotation.",
183+
"Workflow step 0 has no label.",
185184
"Workflow missing test cases.",
186-
"Workflow step with ID 1 specifies an untyped parameter as an input.",
187-
"Workflow step with ID 1 specifies an untyped parameter in the post-job actions.",
185+
"Workflow step 1 specifies an untyped parameter as an input.",
186+
"Workflow step 1 specifies an untyped parameter in the post-job actions.",
188187
]
189188

190189
for warning in warnings:

0 commit comments

Comments
 (0)