Skip to content

Improve type annotation of workflow management methods#22637

Open
nsoranzo wants to merge 4 commits intogalaxyproject:devfrom
nsoranzo:improve_wf_management_type_annot
Open

Improve type annotation of workflow management methods#22637
nsoranzo wants to merge 4 commits intogalaxyproject:devfrom
nsoranzo:improve_wf_management_type_annot

Conversation

@nsoranzo
Copy link
Copy Markdown
Member

@nsoranzo nsoranzo commented May 5, 2026

Also:

  • Fix mypy exclude config

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@github-project-automation github-project-automation Bot moved this to Needs Review in Galaxy Dev - weeklies May 5, 2026
@github-actions github-actions Bot added area/API area/workflows area/database Galaxy's database or data access layer labels May 5, 2026
@github-actions github-actions Bot added this to the 26.1 milestone May 5, 2026
if step.upgrade_messages:
has_upgrade_messages = True
if step.type in ("tool", "subworkflow", None):
assert isinstance(step.module, (ToolModule, SubWorkflowModule))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems surprising in combination with None in the if gaurd ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The None comes from #10121 . My understanding is that if step.type is None, we assume that it's a tool.

Comment thread lib/galaxy/managers/workflows.py
if step_errors:
errors[step.id] = step_errors
if missing_tools:
workflow.annotation = self.get_item_annotation_str(trans.sa_session, trans.user, workflow)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is that because we never serialize the annotation ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The Workflow class/table doesn't have an annotation field (which is flagged by mypy). This comes from #2935 and seems unused.

@nsoranzo nsoranzo force-pushed the improve_wf_management_type_annot branch from 5e3a417 to 85ae996 Compare May 6, 2026 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

2 participants