Conversation
|
| "polars", | ||
| "pandas", | ||
| "pyarrow", | ||
| "black>=26.1.0", |
| variables: | ||
| id_col_expr: | ||
| default: '"id"::varchar' | ||
| age_str_col_expr: | ||
| default: '"age_str"::integer' | ||
| json_col_expr: | ||
| default: "json_col::json->>'unique_id'" | ||
| born_date_col_expr: | ||
| default: '"born_date_str"::DATE' |
There was a problem hiding this comment.
Not expecting any impact on this. Just checking an edge case.
Do we have a test where we test the same "variable" in multiple parts of the contract?
There was a problem hiding this comment.
unsure, we use one mechanism to resolve variables for everything so that should not be a concern 🤔
mivds
left a comment
There was a problem hiding this comment.
Too late for review, but oh well. Something to think about for refactor maybe 🙂
| MaxTimestampMetricImpl( | ||
| contract_impl=contract_impl, | ||
| check_impl=self, | ||
| column=self.column, |
There was a problem hiding this comment.
This change seems odd. I had expected a column_expression line to replace it, but I only see removal
There was a problem hiding this comment.
Hmm, on looking below I see MaxTimestampMetricImpl uses check_impl.column_expression, so it will work... just seems a bit weird design. Why does the metric have a dependency on the check? This seems like another case where the concepts are entangled with a two-way binding between metric and check
There was a problem hiding this comment.
yes it is, but I did not introduce it here, checkImp objects are passed into most(or all?) metrics 👎 Fortunately they are not being saved and used further, just data being extracted...I will still have a small follow up on this as I think I should keep the self.column in the metric identity as before, but yeah, no cleanup of the entanglement for now
Description
Expands the feature to a few more check types. Adds tests.
Black was not present in the list of uv dev packages, so I also added that and updated the lock file.
Accompanying PR for extensions -
Core will need to be bumped in ext after this one is released.
✅ Manual CI run on postgres for these two matching PRs here https://github.com/sodadata/soda-extensions/actions/runs/22398826472
Checklist