galaxyproject / galaxyproject/gxformat2

lint_format2 / lint_ga signature change broke Planemo integration

Open
#187 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
14
Forks
7
Avg merge
2h 49m
Merged PRs (30d)
8

Description

## Summary

Commit `ce4f3c1` ("Migrate lint.py to normalized models, merge best practices") changed the public signatures of `gxformat2.lint.lint_format2` and `gxformat2.lint.lint_ga` in ways that silently broke downstream callers — Planemo in particular.

## Before

```python
def lint_ga(lint_context, workflow_dict, path=None): ...
def lint_format2(lint_context, workflow_dict, path=None): ...
```

## After `ce4f3c1`

```python
def lint_ga(lint_context, nnw: NormalizedNativeWorkflow, raw_dict: dict | None = None): ...
def lint_format2(lint_context, nf2: NormalizedFormat2, raw_dict: dict | None = None): ...
```

Second positional argument switched from a raw `dict` to a normalized pydantic model, and the `path=` keyword argument was removed.

## Downstream impact

[`planemo/workflow_lint.py:166-167`](https://github.com/galaxyproject/planemo/blob/master/planemo/workflow_lint.py#L166-L167):

```python
lint_func = lint_format2 if workflow_class == "GalaxyWorkflow" else lint_ga
lint_func(lint_context, workflow_dict, path=path)
```

Planemo passes a raw dict and a `path=` kwarg. Both fail against the new signature — `TypeError` for `path=`, `AttributeError` deeper in the call chain for the dict.

## Proposed fix

Make the entry points polymorphic and tolerate the legacy kwargs:

```python
def lint_ga(lint_context, nnw, raw_dict=None, path=None):
if isinstance(nnw, dict):
if raw_dict is None:
raw_dict = nnw
nnw = ensure_native(nnw)
...
```

(Same for `lint_format2`.) Keeps existing model-based callers working, restores Planemo. `path=` is accepted and ignored.

Being addressed on branch `abstraction_applications` as part of the linting overhaul.

## Minor secondary issue

`gxformat2.lint` uses `str.format(**kwds)` for one template; `galaxy.tool_util.lint.LintContext` (the context Planemo actually passes) uses `%`-style formatting. Placeholders like `{value}` survive unformatted when the galaxy context is used. Best avoided by using f-strings.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.