search

argoproj-labs / hera

Hera makes Python code easy to orchestrate on Argo Workflows through native Python integrations. It lets you construct and submit your Workflows entirely in Python. ⭐️ Remember to star!
https://hera.rtfd.io
Apache License 2.0
601 stars 106 forks source link

Use overloads in script decorator types #1180

Closed alicederyn closed 2 months ago

alicederyn commented 2 months ago

Pull Request Checklist

Description of PR Currently, @script-decorated user functions return a union of callables, making it impossible to type-check without suppressing mypy's 'call-arg' error (or equivalent), which completely disables parameter type-checking.

This PR changes the type annotations to use an overloaded callable Protocol to select the signature most likely to be what the user was intending to pick.

Unfortunately, the call signatures to Step and Task have to be duplicated, as:

Screenshots

VSCode tooltips

The help text here comes from the docstrings in the new Protocol; to obtain this, I had to suppress ruff's D418 rule.

First VSCode tooltip, showing a no-argument overload that may return a step, a task, or None Second VSCode tooltip, showing an overload that may return a step or task Third VSCode tooltip, showing an overload that will return a task Final VSCode tooltip, showing an overload that will call the user code

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.6%. Comparing base (d553546) to head (d8a847d). Report is 132 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1180 +/- ## ======================================= - Coverage 81.7% 81.6% -0.2% ======================================= Files 54 60 +6 Lines 4208 4767 +559 Branches 889 1020 +131 ======================================= + Hits 3439 3891 +452 - Misses 574 646 +72 - Partials 195 230 +35 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

alicederyn commented 2 months ago

Just spotted that I missed the Workflow overload. Moving back to draft until I fix and test that.