[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Adding range support to groupby

From: Shawn Wagner
Subject: Re: Adding range support to groupby
Date: Fri, 5 Aug 2022 19:33:18 -0700

I'll add in the test changes, no problem.

On Fri, Aug 5, 2022 at 2:06 PM Tim Rice <trice@posteo.net> wrote:
Hey Shawn,

On Fri, Aug 05, 2022 at 02:42:12AM -0700, Shawn Wagner wrote:
>Attached patch allows for usage like `groupby 1-4` instead of `groupby
>1,2,3,4`. Thoughts before I commit it?

Cool idea. I had a couple of very minor nits like some of the column alignment in the tests was off. It does make it easier to eyeball tests when everything lines up just so. Also, c3 in the crosstab tests shouldn't be changed, but instead there should be a new test there.

Then I thought that we should also test having multiple columns instead of just two.

I've attached a patch which addresses those thoughts.

Let's wait for Erik to also weigh in before committing it. He has a good eye for detail, and might notice additional things that will make it better.

By the way, testing the range out on crosstab helped me notice another minor bug. If the output has wide columns (eg when using unique), crosstab's column alignment ends up all out of whack. I've added a note to myself to fix that sometime. It might be difficult, though.

~ Tim

reply via email to

[Prev in Thread] Current Thread [Next in Thread]