calogica / dbt-expectations

Port(ish) of Great Expectations to dbt test macros
https://calogica.github.io/dbt-expectations/
Apache License 2.0
1.06k stars 130 forks source link

Error in macros/schema_tests/string_matching/* #125

Closed samantha-guerriero-cko closed 2 years ago

samantha-guerriero-cko commented 2 years ago

All the methods in macros/schema_tests/string_matching/ define a group_by_columns=group_by functionality, but groupby is not defined in the method which makes them fail with **'MacroGenerator' object is not iterable_**.

Fix should be to set up group_by=None in methods' definition as per the other macros with this functionality.

e.g., {% test expect_column_value_lengths_to_equal(model, column_name, value, group_by=None, row_condition=None ) %}

clausherther commented 2 years ago

Hi @samantha-guerriero-cko , thanks for raising that issue! You're right, I see the missing params. What's weird is that this passes our integration tests without an error. For example, this test gets called here. Could you share some more information about your environment (which platform, what version dbt & Python)?

Also, let me know if you're interested in submitting a PR for this before I jump on this myself.

clausherther commented 2 years ago

Btw, I think the correct treatment here is probably to not support group_by for this particular test, since the expression is not an aggregation so it would fail on the group by anyway.

So, I'd set the group_by_columns param to None in the call to expression_is_true:

{{ dbt_expectations.expression_is_true(model,
                                        expression=expression,
                                        group_by_columns=None,
                                        row_condition=row_condition
                                        )
                                        }}
samantha-guerriero-cko commented 2 years ago

Btw, I think the correct treatment here is probably to not support group_by for this particular test, since the expression is not an aggregation so it would fail on the group by anyway.

So, I'd set the group_by_columns param to None in the call to expression_is_true:

{{ dbt_expectations.expression_is_true(model,
                                        expression=expression,
                                        group_by_columns=None,
                                        row_condition=row_condition
                                        )
                                        }}

It makes sense to make the group_by_columns=None :)

For what concerns my environment:

I am happy to open a PR for this 💪

clausherther commented 2 years ago

@samantha-guerriero-cko amazing, thank you!