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

Airflow integration #1412

Merged
merged 10 commits into from May 15, 2022
Merged

Airflow integration #1412

merged 10 commits into from May 15, 2022

Conversation

ItayGabbay
Copy link
Member

@ItayGabbay ItayGabbay commented May 10, 2022

Reference Issues/PRs

Closes #1411

What does this implement/fix? Explain your changes.

Adding an integration with apache airflow.

image
image
image


Thanks for contributing a pull request! Please ensure you have taken a look at
the contribution guidelines: https://github.com/deepchecks/deepchecks/blob/main/CONTRIBUTING.rst

@ItayGabbay ItayGabbay added the feature Feature update or code change to the package label May 10, 2022
@ItayGabbay ItayGabbay requested a review from shir22 as a code owner May 10, 2022 13:32
@ItayGabbay ItayGabbay self-assigned this May 10, 2022
@ItayGabbay ItayGabbay requested a review from a team as a code owner May 10, 2022 13:32
@matanper
Copy link
Contributor

I think the DAG would be more realistic if it had simple model training as a step before the checks. but if it's too complex this is also fine

docs/source/user-guide/integrations/airflow.rst Outdated Show resolved Hide resolved
docs/source/user-guide/integrations/airflow.rst Outdated Show resolved Hide resolved
docs/source/user-guide/integrations/airflow.rst Outdated Show resolved Hide resolved
docs/source/user-guide/integrations/airflow.rst Outdated Show resolved Hide resolved
docs/source/user-guide/integrations/airflow.rst Outdated Show resolved Hide resolved
docs/source/user-guide/integrations/airflow.rst Outdated Show resolved Hide resolved
@@ -0,0 +1,115 @@
from datetime import datetime, timedelta
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Is it possible for the rst file to take "code blocks" as lines from here?
    (so that in the document it will be a reference to line numbers instead of copy-paste of code...?)
    Worth checking (and if so implementing) for finding a good and maintainable solution for the integration tutorial status (that is currently double...)
  2. What's the purpose of this file?
    (Is it quite easy to run it as-is? can we test it?
    and do we think it's simpler than copy-pasting from the docs these code blocks?)

Copy link
Member Author

Choose a reason for hiding this comment

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

For 1 - unfortunately, I can't see a good maintainable solution here. If we want explanations within the code it can be an ipynb, and then refer the doc page to a Colab link. If it's a simple Python script - then probably have it inside the RST. I know it's not maintainable, will have to decide what's the best way to do that.

2 - in order to get it tested we need airflow as a dependency, which I don't think will be a good idea. The script itself is quite easy to run assuming you have airflow installed.

examples/integrations/airflow/README.rst Outdated Show resolved Hide resolved
ItayGabbay and others added 3 commits May 11, 2022 09:20
Co-authored-by: shir22 <33841818+shir22@users.noreply.github.com>
@ItayGabbay ItayGabbay requested a review from shir22 May 11, 2022 06:45
@noamzbr noamzbr mentioned this pull request May 12, 2022
@ItayGabbay ItayGabbay merged commit 9ae6f18 into main May 15, 2022
@delete-merged-branch delete-merged-branch bot deleted the airflow-integration branch May 15, 2022 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature update or code change to the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] Airflow Integration
4 participants