circuithub / rel8

Hey! Hey! Can u rel8?
https://rel8.readthedocs.io
Other
150 stars 38 forks source link

Use less Opaleye internals in aggregation #204

Closed tomjaguarpaw closed 1 year ago

tomjaguarpaw commented 1 year ago

Would it be OK to do this, to depend on less Opaleye internals?

tomjaguarpaw commented 1 year ago

Hey folks, any thoughts on this? This would allow me to rearrange the Opaleye aggregate internals, which are in need of clarification.

ocharles commented 1 year ago

@shane-circuithub Could you take a look at this?

shane-circuithub commented 1 year ago

I'm generally in favour of this. I know we currently don't use the ordering field of our Aggregator type, but we have used it in the past. Is there an equivalent of distinctAggregator for adding an ordering @tomjaguarpaw ?

tomjaguarpaw commented 1 year ago

Are you looking for orderAggregate?

shane-circuithub commented 1 year ago

Yep, that looks like what I mean. In that case I don't see any downside to this.

ocharles commented 1 year ago

Thanks @shane-circuithub!