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

Error in macros/schema_tests/string_matching/* #125

Closed
samantha-guerriero-cko opened this issue Nov 16, 2021 · 4 comments · Fixed by #126
Closed

Error in macros/schema_tests/string_matching/* #125

samantha-guerriero-cko opened this issue Nov 16, 2021 · 4 comments · Fixed by #126
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@samantha-guerriero-cko
Copy link
Contributor

All the methods in macros/schema_tests/string_matching/ define a group_by_columns=group_by functionality, but group_by 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
Copy link
Contributor

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 clausherther added bug Something isn't working good first issue Good for newcomers labels Nov 16, 2021
@clausherther
Copy link
Contributor

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
Copy link
Contributor Author

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:

  • dbt = 0.21.0 (have tried also with 0.20.0)
  • Python = 3.9.6
  • Platform = macOS Big Sur 11.6

I am happy to open a PR for this 💪

@clausherther
Copy link
Contributor

@samantha-guerriero-cko amazing, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants