Skip to content

feat: remove 'smart' double way of decorating tasks and DAGs in favor of a single way that plays better with IDEs#29

Merged
larribas merged 2 commits intomainfrom
type-hints-for-dsl-decorators
Sep 9, 2021
Merged

feat: remove 'smart' double way of decorating tasks and DAGs in favor of a single way that plays better with IDEs#29
larribas merged 2 commits intomainfrom
type-hints-for-dsl-decorators

Conversation

@larribas
Copy link
Copy Markdown
Owner

@larribas larribas commented Sep 9, 2021

What this PR does / why do we need it:

We noticed that certain IDEs were complaining about the types returned by the dsl.DAG and dsl.task decorators.

At the moment, the same function can be used as a decorator, and as a function that returns a decorator.

This double behavior is achieved with some "smart" if condition that adds complexity to the code and the type system.

This PR simplifies the decorators and adds type hints to them. The result may be a bit more verbose, but it is simpler for both users and maintainers of the library.

Which issue(s) does this PR fix:

Fixes #28

Release Notes

Modified dsl decorators so that they are always parameterized (even if the list of parameters is empty).

What type of changes to the API does this change introduce?

MAJOR: This PR contains backwards-incompatible changes to the API (e.g. a new required argument was added to a public method)

Checklist

  • I've read the Contribution Guidelines
  • Updated the appropriate sections of the documentation.
  • I've added automated tests covering all success and error scenarios.

… of a single way that plays better with IDEs
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 9, 2021

Codecov Report

Merging #29 (24fc993) into main (d3574b6) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #29      +/-   ##
==========================================
- Coverage   99.92%   99.92%   -0.01%     
==========================================
  Files          53       53              
  Lines        1300     1299       -1     
==========================================
- Hits         1299     1298       -1     
  Misses          1        1              
Impacted Files Coverage Δ
dagger/dsl/build.py 98.79% <ø> (ø)
dagger/dsl/node_invocation_recorder.py 100.00% <ø> (ø)
dagger/dsl/node_output_key_usage.py 100.00% <ø> (ø)
dagger/dsl/node_output_partition_usage.py 100.00% <ø> (ø)
dagger/dsl/node_output_property_usage.py 100.00% <ø> (ø)
dagger/dsl/node_output_reference.py 100.00% <ø> (ø)
dagger/dsl/node_output_usage.py 100.00% <ø> (ø)
dagger/dsl/dsl.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3574b6...24fc993. Read the comment docs.

@larribas larribas merged commit d1df5e6 into main Sep 9, 2021
@larribas larribas deleted the type-hints-for-dsl-decorators branch September 9, 2021 18:54
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.

IDEs complain about the type returned by the @dsl. decorators

2 participants