|
From: | Erik Auerswald |
Subject: | Re: Adding range support to groupby |
Date: | Sat, 6 Aug 2022 18:28:12 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 |
Hi Shawn, On 06.08.22 15:39, Shawn Wagner wrote:
On Sat, Aug 6, 2022 at 3:36 AM Erik Auerswald <auerswal@unix-ag.uni-kl.de> wrote:Hi Shawn and Tim, On 05.08.22 23:06, Tim Rice wrote: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?[...]Since this affects "crosstab" aka "ct" too, that should be mentioned in NEWS as well.crosstab and groupby use the same function for getting columns; that's the only reason it's affected. Given it only works with two columns I didn't see a point to mentioning it in the documentation, as doing so might make people think it's supposed to work with more.
Yes, just adding "and crosstab" to the NEWS entry would not work. I do not have a good suggestion either, so I am fine with keeping your proposed NEWS entry as is.
[...]I concur with Tim with keeping the 'c3' test and adding a new one based on 'c3' that only replaces "1,2" with "1-2". Then there should be a test that verifies that "crosstab 1-3" results in an error since crosstab requires exactly two fields. There are tests with just one and four fields in tests/datamash-crosstab.pl, I think it could be added near those tests.Sounds good.
:-)
[...]
Thanks, Erik
[Prev in Thread] | Current Thread | [Next in Thread] |