Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GeneratorStep.process validation in DAG and smaller fixes #435

Merged
merged 13 commits into from
Mar 18, 2024

Conversation

alvarobartt
Copy link
Member

Description

This PR adds a method within DAG named _validate_generator_step_process_signature that validates that the provided process method signature matches the expected one, which should contain the offset arg, and also the inputs arg should not be there, otherwise, a ValueError will be raised during the validation.

Closes #431

Additionally, this PR also tackles the following:

  • Add missing CombineColumns docstrings
  • Use Yields over Returns in docstrings where yield is used instead of return
  • Move Task specific methods out of abstract _Task, to avoid conflicts / collisions with GeneratorTask that has a different process signature

@alvarobartt alvarobartt added this to the 1.0.0 milestone Mar 18, 2024
@alvarobartt alvarobartt self-assigned this Mar 18, 2024
@gabrielmbmb
Copy link
Member

Hi, LGTM! I made some small changes to reuse Step logic instead of calling inspect.signature again, and to check there is no parameter type hinted with StepInput, as that's the important thing not the name. BTW, it's important to not import StepInput within a TYPE_CHECKING block otherwise we cannot check it's a parameter annotated with the metadata we need.

To be able to use the runtime type-hints we will need to put `StepInput` outsie the `TYPE_CHECKING`

Co-authored-by: Gabriel Martin <gabrielmbmb@users.noreply.github.com>
@alvarobartt alvarobartt merged commit 9622d4c into core-refactor Mar 18, 2024
4 checks passed
@alvarobartt alvarobartt deleted the generator-step-safe-check branch March 18, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Raise RuntimeError if GeneratorStep.process signature contains inputs
2 participants