Control operation combinations #147
Conversation
Prevent aggregation operations besides First and Last to be paired with order by
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #147 +/- ##
==========================================
+ Coverage 90.36% 90.43% +0.07%
==========================================
Files 25 25
Lines 934 941 +7
==========================================
+ Hits 844 851 +7
Misses 90 90
☔ View full report in Codecov by Sentry. |
| assert { | ||
| filter_op.__class__.__name__, | ||
| transform_op.__class__.__name__, | ||
| agg_op.__class__.__name__, | ||
| }.intersection(filter_op.restricted_ops) == set() |
There was a problem hiding this comment.
| assert { | |
| filter_op.__class__.__name__, | |
| transform_op.__class__.__name__, | |
| agg_op.__class__.__name__, | |
| }.intersection(filter_op.restricted_ops) == set() | |
| filter_cls_name = filter_op.__class__.__name__ | |
| transform_cls_name = transform_op.__class__.__name__ | |
| agg_cls_name = agg_op.__class__.__name__ | |
| assert { | |
| filter_cls_name, | |
| transform_cls_name, | |
| agg_cls_name, | |
| }.intersection(filter_op.restricted_ops) == set() |
There was a problem hiding this comment.
you can also use the variables below
| transform_operation.__name__, | ||
| filter_operation.__name__, | ||
| }.intersection( | ||
| agg_operation.restricted_ops.union( |
There was a problem hiding this comment.
restricted_ops could be converted to the actual python objects, rather than relying on strings
There was a problem hiding this comment.
I had circular dependency issues when I tried this (since in agg ops I am restricting transformation ops, and in transformation ops I am restricting agg ops; so I need to import one into another and vice versa), do you have an idea on how to approach it?
There was a problem hiding this comment.
I definitely agree with this though
There was a problem hiding this comment.
oh I see. Let's leave it for now that, as is
FirstAggregationOpandLastAggregationOpto be paired withOrderByOpFirstAggregationOpandLastAggregationOpfrom being paired withIdentityByOp(hence only allowingIdentityByOp)