Conversation
|
senbong seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Experiment ResultsExperiment 1: air-passengersDescription:
Results:
Plot:Experiment 2: air-passengersDescription:
Results:
Plot:Experiment 3: electricity-multiple-seriesDescription:
Results:
Plot:Experiment 4: electricity-multiple-seriesDescription:
Results:
Plot:Experiment 5: electricity-multiple-seriesDescription:
Results:
Plot: |
| MOD(HASH(unique_id), :MAX_BATCHES) AS gp, | ||
| unique_id, | ||
| ds, | ||
| OBJECT_CONSTRUCT_KEEP_NULL(*) AS data_obj |
There was a problem hiding this comment.
OBJECT_CONSTRUCT_KEEP_NULL is used here to handle the case when the historical exog variable has null value. Previous implementation:
|
|
||
| ╔══════════════════════════════════════════════════════════════════════════════╗ | ||
| ║ 🎉 Ready to start forecasting! ║ | ||
| ╚══════════════════════════════════════════════════════════════════════════════╝ |
There was a problem hiding this comment.
Sample scripts are generated to show user how to call the stored procedures using the generated sample tables.
Makefile
Outdated
|
|
||
| format: | ||
| @echo "Running black formatter on staged files..." | ||
| @git diff --cached --name-only --diff-filter=ACMR | grep '\.py$$' | xargs -r uv run black No newline at end of file |
There was a problem hiding this comment.
Suggestion: Deployment execution command like
python -m nixtla.scripts.snowflake_install_nixtla can be registered as part of Makefile command.
| CREATE OR REPLACE NETWORK RULE {ds_prefix}nixtla_network_rule | ||
| MODE = EGRESS | ||
| TYPE = HOST_PORT | ||
| VALUE_LIST = ('api.nixtla.io'); |
There was a problem hiding this comment.
Please help to make this an option that user can customize the domain name, in case we want to use a value other than 'api.nixtla.io'
| ``` | ||
| """ | ||
|
|
||
| PACKAGES = [ |
There was a problem hiding this comment.
Suggest that the essential packages can be read from the pyproject.toml and on top of that, we add the additional snowflake required dependencies.
The objective is that if Nixtla client has newly introduced dependency, we can automatically consider them than updating this manually for snowflake integration.
There was a problem hiding this comment.
This is exactly the thing that didn't work in snowflake. Snowflake running environment is extremely picky on libraries, it doesn't support many of them, so we have to carefully pick the ones. I spent the most time to figure out this list when I worked on this. But @gee-senbong feel free to try what @JQGoh suggests. Maybe it's much easier to work with now.
Makefile
Outdated
|
|
||
| format: | ||
| @echo "Running black formatter on staged files..." | ||
| @git diff --cached --name-only --diff-filter=ACMR | grep '\.py$$' | xargs -r uv run black No newline at end of file |
There was a problem hiding this comment.
i think we should move to uv format but these two new commands are good to add.
There was a problem hiding this comment.
Will update this to use uv format.
af579ec to
245981b
Compare
| dependencies = [ | ||
| "annotated-types", | ||
| "httpx[zstd]", | ||
| "narwhals>=2.11.0", |
There was a problem hiding this comment.
Update this to fix the new integration test error when running in python 3.9 environment:
> sf_df, client_df = self._execute_and_compare(
snowflake_session,
deployed_with_api_endpoint,
example_dataframes,
"evaluation_metrics",
compare_evaluate,
)
nixtla_tests/snowflake/test_snowflake_deployment.py:379:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
nixtla_tests/snowflake/test_snowflake_deployment.py:218: in _execute_and_compare
client_df = comparison_fn(test_case, example_data[test_case.input_table])
nixtla_tests/snowflake/test_snowflake_deployment.py:364: in compare_evaluate
result = evaluate(data, metrics=metrics, models=forecasters, train_df=data)
.venv/lib/python3.9/site-packages/utilsforecast/evaluation.py:314: in evaluate
result = metric(**kwargs)
.venv/lib/python3.9/site-packages/utilsforecast/losses.py:331: in mape
return _nw_agg_expr(
.venv/lib/python3.9/site-packages/utilsforecast/losses.py:76: in _nw_agg_expr
nw.from_native(df)
.venv/lib/python3.9/site-packages/narwhals/dataframe.py:1519: in select
return super().select(*exprs, **named_exprs)
.venv/lib/python3.9/site-packages/narwhals/dataframe.py:232: in select
return self._with_compliant(self._compliant_frame.select(*compliant_exprs))
.venv/lib/python3.9/site-packages/narwhals/_pandas_like/dataframe.py:421: in select
new_series = self._evaluate_into_exprs(*exprs)
.venv/lib/python3.9/site-packages/narwhals/_compliant/dataframe.py:360: in _evaluate_into_exprs
return tuple(
.venv/lib/python3.9/site-packages/narwhals/_compliant/dataframe.py:361: in <genexpr>
chain.from_iterable(self._evaluate_into_expr(expr) for expr in exprs) # pyright: ignore[reportArgumentType]
.venv/lib/python3.9/site-packages/narwhals/_compliant/dataframe.py:375: in _evaluate_into_expr
result = expr(self)
.venv/lib/python3.9/site-packages/narwhals/_compliant/expr.py:236: in __call__
return self._call(df)
.venv/lib/python3.9/site-packages/narwhals/_compliant/expr.py:675: in <lambda>
lambda df: [series.alias(name) for series in self(df)],
.venv/lib/python3.9/site-packages/narwhals/_compliant/expr.py:236: in __call__
return self._call(df)
.venv/lib/python3.9/site-packages/narwhals/_compliant/expr.py:375: in _reuse_series_inner
**{
.venv/lib/python3.9/site-packages/narwhals/_compliant/expr.py:376: in <dictcomp>
name: df._evaluate_expr(value) if self._is_expr(value) else value
.venv/lib/python3.9/site-packages/narwhals/_compliant/dataframe.py:352: in _evaluate_expr
result: Sequence[EagerSeriesT] = expr(self)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = PandasThen(depth=0, function_name=whenthen)
df = <narwhals._pandas_like.dataframe.PandasLikeDataFrame object at 0x137e37730>
def __call__(self, df: EagerDataFrameT) -> Sequence[EagerSeriesT]:
> return self._call(df)
E AttributeError: 'PandasThen' object has no attribute '_call'
a148893 to
5b1f7b4
Compare
c2955a9 to
d28015f
Compare
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
8eb2361 to
cdc59f9
Compare
9f47f80 to
3f3463b
Compare
1ac0b3e to
fc9e1f1
Compare
goodwanghan
left a comment
There was a problem hiding this comment.
We have had a lot of offline discussion, and the PR reflects the design decisions based on our discussion.





This PR revises previous PR (#648) to add snowflake integration support.
Command to trigger the script to install Snowflake stored procedure: