You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "vinay2242g (via GitHub)" <gi...@apache.org> on 2024/05/22 18:10:50 UTC
[PR] Update auto_ml.py [airflow]
vinay2242g opened a new pull request, #39767:
URL: https://github.com/apache/airflow/pull/39767
Added window_stride_length & window_max_count
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding params. to create_auto_ml_forecasting_training_job in AutoMl hook [airflow]
Posted by "vinay2242g (via GitHub)" <gi...@apache.org>.
vinay2242g commented on PR #39767:
URL: https://github.com/apache/airflow/pull/39767#issuecomment-2127381765
Thank you Shahar,
Could you please let me if there any action item on my side?
Thanks,
Siddesh
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding params. to create_auto_ml_forecasting_training_job in AutoMl hook [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on PR #39767:
URL: https://github.com/apache/airflow/pull/39767#issuecomment-2129456268
> Thank you Shahar,
>
> Could you please let me if there any action item on my side?
>
> Thanks, Siddesh
It looks good to me, but we need approval from a committer so it can be merged.
@eladkal we'd be happy for your help :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update auto_ml.py [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #39767:
URL: https://github.com/apache/airflow/pull/39767#issuecomment-2125452799
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding params. to create_auto_ml_forecasting_training_job in AutoMl hook [airflow]
Posted by "vinay2242g (via GitHub)" <gi...@apache.org>.
vinay2242g commented on PR #39767:
URL: https://github.com/apache/airflow/pull/39767#issuecomment-2126656220
Thank you Shahar,
I have tried to implement things that you have mentioned.
Could you please let me know if they look good.
Thanks,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update auto_ml.py [airflow]
Posted by "vinay2242g (via GitHub)" <gi...@apache.org>.
vinay2242g commented on PR #39767:
URL: https://github.com/apache/airflow/pull/39767#issuecomment-2125494348
Hi,
I am really really new to open source community. I may not be fully aware lot of things here.
Please help me grow here.
Thanks,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding params. to create_auto_ml_forecasting_training_job in AutoMl hook [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #39767:
URL: https://github.com/apache/airflow/pull/39767#discussion_r1613803574
##########
airflow/providers/google/cloud/operators/vertex_ai/auto_ml.py:
##########
@@ -170,6 +172,8 @@ def __init__(
self.quantiles = quantiles
self.validation_options = validation_options
self.budget_milli_node_hours = budget_milli_node_hours
+ self.window_stride_length = window_stride_length,
+ self.window_max_count = window_max_count,
Review Comment:
And also suggested changes will fix the tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update auto_ml.py [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on code in PR #39767:
URL: https://github.com/apache/airflow/pull/39767#discussion_r1610950987
##########
airflow/providers/google/cloud/hooks/vertex_ai/auto_ml.py:
##########
@@ -551,6 +551,8 @@ def create_auto_ml_forecasting_training_job(
is_default_version: bool | None = None,
model_version_aliases: list[str] | None = None,
model_version_description: str | None = None,
+ window_stride_length: [int] | None = None,
+ window_max_count: [int] | None = None,
Review Comment:
```suggestion
window_stride_length: int | None = None,
window_max_count: int | None = None,
```
##########
airflow/providers/google/cloud/hooks/vertex_ai/auto_ml.py:
##########
@@ -703,6 +705,10 @@ def create_auto_ml_forecasting_training_job(
:param sync: Whether to execute this method synchronously. If False, this method will be executed in
concurrent Future and any downstream object will be immediately returned and synced when the
Future has completed.
+ :window_stride_length: Optional. Step length used to generate input examples. Every
+ ``window_stride_length`` rows will be used to generate a sliding window.
+ window_max_count: Optional. Number of rows that should be used to generate input examples. If the
+ total row count is larger than this number, the input data will be randomly sampled to hit the count.
Review Comment:
```suggestion
:param window_stride_length: Optional. Step length used to generate input examples. Every
``window_stride_length`` rows will be used to generate a sliding window.
:param window_max_count: Optional. Number of rows that should be used to generate input examples. If the
total row count is larger than this number, the input data will be randomly sampled to hit the count.
```
##########
airflow/providers/google/cloud/hooks/vertex_ai/auto_ml.py:
##########
@@ -758,6 +764,8 @@ def create_auto_ml_forecasting_training_job(
is_default_version=is_default_version,
model_version_aliases=model_version_aliases,
model_version_description=model_version_description,
+ window_stride_length=window_stride_length,
+ window_max_count=window_stride_length,
Review Comment:
```suggestion
window_max_count=window_max_count,
```
##########
airflow/providers/google/cloud/operators/vertex_ai/auto_ml.py:
##########
@@ -220,6 +224,8 @@ def execute(self, context: Context):
model_display_name=self.model_display_name,
model_labels=self.model_labels,
sync=self.sync,
+ window_stride_length=self.window_stride_length,
+ window_max_count=self.window_stride_length,
Review Comment:
```suggestion
window_stride_length=self.window_stride_length,
window_max_count=self.window_max_count,
```
##########
airflow/providers/google/cloud/operators/vertex_ai/auto_ml.py:
##########
@@ -138,6 +138,8 @@ def __init__(
region: str,
impersonation_chain: str | Sequence[str] | None = None,
parent_model: str | None = None,
+ window_stride_length: [int] | None = None,
+ window_max_count: [int] | None = None,
Review Comment:
```suggestion
window_stride_length: int | None = None,
window_max_count: int | None = None,
```
##########
airflow/providers/google/cloud/operators/vertex_ai/auto_ml.py:
##########
@@ -170,6 +172,8 @@ def __init__(
self.quantiles = quantiles
self.validation_options = validation_options
self.budget_milli_node_hours = budget_milli_node_hours
+ self.window_stride_length=window_stride_length,
+ self.window_max_count=window_stride_length,
Review Comment:
```suggestion
self.window_stride_length=window_stride_length,
self.window_max_count=window_max_count,
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding params. to create_auto_ml_forecasting_training_job in AutoMl hook [airflow]
Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell merged PR #39767:
URL: https://github.com/apache/airflow/pull/39767
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding params. to create_auto_ml_forecasting_training_job in AutoMl hook [airflow]
Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #39767:
URL: https://github.com/apache/airflow/pull/39767#discussion_r1613655508
##########
airflow/providers/google/cloud/hooks/vertex_ai/auto_ml.py:
##########
@@ -703,6 +705,10 @@ def create_auto_ml_forecasting_training_job(
:param sync: Whether to execute this method synchronously. If False, this method will be executed in
concurrent Future and any downstream object will be immediately returned and synced when the
Future has completed.
+ :param window_stride_length: Optional. Step length used to generate input examples. Every
+ ``window_stride_length`` rows will be used to generate a sliding window.
+ :param window_max_count: Optional. Number of rows that should be used to generate input examples. If the
+ total row count is larger than this number, the input data will be randomly sampled to hit the count.
Review Comment:
```suggestion
:param window_stride_length: Optional. Step length used to generate input examples. Every
``window_stride_length`` rows will be used to generate a sliding window.
:param window_max_count: Optional. Number of rows that should be used to generate input examples. If the
total row count is larger than this number, the input data will be randomly sampled to hit the count.
```
Small nit on [formatting the docstring params](https://sphinx-rtd-tutorial.readthedocs.io/en/latest/docstrings.html#an-example-class-with-docstrings) in the Sphinx RTD format for the [Python API docs](https://airflow.apache.org/docs/apache-airflow-providers-google/stable/_api/airflow/providers/google/cloud/hooks/vertex_ai/auto_ml/index.html#airflow.providers.google.cloud.hooks.vertex_ai.auto_ml.AutoMLHook.create_auto_ml_forecasting_training_job).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding params. to create_auto_ml_forecasting_training_job in AutoMl hook [airflow]
Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on PR #39767:
URL: https://github.com/apache/airflow/pull/39767#issuecomment-2132231247
Congrats @vinay2242g 🎉
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding params. to create_auto_ml_forecasting_training_job in AutoMl hook [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #39767:
URL: https://github.com/apache/airflow/pull/39767#discussion_r1613803574
##########
airflow/providers/google/cloud/operators/vertex_ai/auto_ml.py:
##########
@@ -170,6 +172,8 @@ def __init__(
self.quantiles = quantiles
self.validation_options = validation_options
self.budget_milli_node_hours = budget_milli_node_hours
+ self.window_stride_length = window_stride_length,
+ self.window_max_count = window_max_count,
Review Comment:
And also fix the tests
##########
airflow/providers/google/cloud/operators/vertex_ai/auto_ml.py:
##########
@@ -170,6 +172,8 @@ def __init__(
self.quantiles = quantiles
self.validation_options = validation_options
self.budget_milli_node_hours = budget_milli_node_hours
+ self.window_stride_length = window_stride_length,
+ self.window_max_count = window_max_count,
Review Comment:
```suggestion
self.window_stride_length = window_stride_length
self.window_max_count = window_max_count
```
In this case comma in the end transform into the tuples `(window_stride_length, )` and `(window_max_count, )`
I guess it added accidentally
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update auto_ml.py [airflow]
Posted by "vinay2242g (via GitHub)" <gi...@apache.org>.
vinay2242g closed pull request #39767: Update auto_ml.py
URL: https://github.com/apache/airflow/pull/39767
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding params. to create_auto_ml_forecasting_training_job in AutoMl hook [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #39767:
URL: https://github.com/apache/airflow/pull/39767#issuecomment-2132227676
Awesome work, congrats on your first merged pull request! You are invited to check our [Issue Tracker](https://github.com/apache/airflow/issues) for additional contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org