digitalmars.D - Another regression: std.range.slide
- H. S. Teoh (30/30) Jun 15 2023 https://issues.dlang.org/show_bug.cgi?id=23976
- Walter Bright (2/5) Jun 15 2023 FeepingCreature is aware of this and says he'll look into it tomorrow.
- H. S. Teoh (8/14) Jun 15 2023 Sometimes I wonder if I should put my projects on github and add it to
- Walter Bright (3/7) Jun 18 2023 Maybe you should! But you would also need to be willing to help isolate ...
- FeepingCreature (5/15) Jun 18 2023 Ohey, while I have your attention: any input on
- FeepingCreature (22/51) Jun 16 2023 I actually need some help with this.
- Sergey (4/8) Jun 16 2023 It seems withPartial thing should work only when the specific
- FeepingCreature (2/11) Jun 16 2023 Sure, but in these cases isn't the window specified as 2?
- Sergey (5/18) Jun 16 2023 I would say if it is not specified, than it is equal to
- H. S. Teoh (56/64) Jun 21 2023 [...]
- jmh530 (81/86) Jun 21 2023 You can go pretty far with unittest examples.
- H. S. Teoh (7/13) Jun 21 2023 [...]
https://issues.dlang.org/show_bug.cgi?id=23976 Caused by: https://github.com/dlang/phobos/pull/8738 (commit 8a9cfa2677) Brief summary: ----------- import std; void main() { auto input = "1<2"; foreach (pair; input.splitter("<").slide(2)) { writeln(pair); } } ----------- Output before commit 8a9cfa2677: ----------- ["1", "2"] ----------- Output after commit 8a9cfa2677: ----------- ["1", "2"] ["2"] ----------- This is incorrect because the code asked for a window of size 2, with the default option of No.withPartial. This broke another of my old projects that relied on the old (correct) behaviour. :-/ T -- There are three kinds of people in the world: those who can count, and those who can't.
Jun 15 2023
On 6/15/2023 12:18 PM, H. S. Teoh wrote:https://issues.dlang.org/show_bug.cgi?id=23976 Caused by: https://github.com/dlang/phobos/pull/8738FeepingCreature is aware of this and says he'll look into it tomorrow.
Jun 15 2023
On Thu, Jun 15, 2023 at 06:18:48PM -0700, Walter Bright via Digitalmars-d wrote:On 6/15/2023 12:18 PM, H. S. Teoh wrote:Sometimes I wonder if I should put my projects on github and add it to the Phobos CI. :-D It seems I have quite a bit of code that triggers unusual cases or use things in rare combinations that tend to break when something gets changed. T -- Не дорог подарок, дорога любовь.https://issues.dlang.org/show_bug.cgi?id=23976 Caused by: https://github.com/dlang/phobos/pull/8738FeepingCreature is aware of this and says he'll look into it tomorrow.
Jun 15 2023
On 6/15/2023 9:18 PM, H. S. Teoh wrote:Sometimes I wonder if I should put my projects on github and add it to the Phobos CI. :-D It seems I have quite a bit of code that triggers unusual cases or use things in rare combinations that tend to break when something gets changed.Maybe you should! But you would also need to be willing to help isolate problems when they occur building that code.
Jun 18 2023
FeepingCreature <feepingcreature gmail.com> On Sunday, 18 June 2023 at 17:47:35 UTC, Walter Bright wrote:On 6/15/2023 9:18 PM, H. S. Teoh wrote:Ohey, while I have your attention: any input on https://forum.dlang.org/post/ivmrolypalfsoqeqecki forum.dlang.org ? Maybe we should just revert the original fix. Any observable behavior is a feature, and so on.Sometimes I wonder if I should put my projects on github and add it to the Phobos CI. :-D It seems I have quite a bit of code that triggers unusual cases or use things in rare combinations that tend to break when something gets changed.Maybe you should! But you would also need to be willing to help isolate problems when they occur building that code.
Jun 18 2023
On Thursday, 15 June 2023 at 19:18:03 UTC, H. S. Teoh wrote:https://issues.dlang.org/show_bug.cgi?id=23976 Caused by: https://github.com/dlang/phobos/pull/8738 (commit 8a9cfa2677) Brief summary: ``` import std; void main() { auto input = "1<2"; foreach (pair; input.splitter("<").slide(2)) { writeln(pair); } } ``` Output before commit 8a9cfa2677: ``` ["1", "2"] ``` Output after commit 8a9cfa2677: ``` ["1", "2"] ["2"] ``` This is incorrect because the code asked for a window of size 2, with the default option of No.withPartial. This broke another of my old projects that relied on the old (correct) behaviour. :-/ TI actually need some help with this. The behavior of `withPartial` seems to be random! For instance, since the default of `slide` is stated to be `Yes.withPartial`, shouldn't `[["1", "2"], ["2"]]` be the correct answer, since the last element has less than two entries? But then the unittests on https://dlang.org/library/std/range/slide.html are all over the place. For instance: ``` assert([0, 1, 2, 3].slide(2).equal!equal( [[0, 1], [1, 2], [2, 3]] )); ``` But this is `Yes.withPartial` too! So shouldn't it end with `, [3]`? Either I'm severely not getting something here, or the bug report is in fact the wrong way around and the bug is that `slide` sometimes *doesn't* append the partial last window. Do we just need to revert my PR, throw up our hands and admit that the behavior of `slide` is just basically down to the vagaries of range semantics, and start over with `slide2`?
Jun 16 2023
On Friday, 16 June 2023 at 08:41:48 UTC, FeepingCreature wrote:The behavior of `withPartial` seems to be random! For instance, since the default of `slide` is stated to be `Yes.withPartial`, shouldn't `[["1", "2"], ["2"]]` be the correct answer, since the last element has less than two entries?It seems withPartial thing should work only when the specific Window specified. Because in other case the whole range is available.
Jun 16 2023
On Friday, 16 June 2023 at 09:32:38 UTC, Sergey wrote:On Friday, 16 June 2023 at 08:41:48 UTC, FeepingCreature wrote:Sure, but in these cases isn't the window specified as 2?The behavior of `withPartial` seems to be random! For instance, since the default of `slide` is stated to be `Yes.withPartial`, shouldn't `[["1", "2"], ["2"]]` be the correct answer, since the last element has less than two entries?It seems withPartial thing should work only when the specific Window specified. Because in other case the whole range is available.
Jun 16 2023
On Friday, 16 June 2023 at 09:40:13 UTC, FeepingCreature wrote:On Friday, 16 June 2023 at 09:32:38 UTC, Sergey wrote:I would say if it is not specified, than it is equal to range.length But I'm not using slide too often. Maybe someone else could elaborate more on this.On Friday, 16 June 2023 at 08:41:48 UTC, FeepingCreature wrote:Sure, but in these cases isn't the window specified as 2?The behavior of `withPartial` seems to be random! For instance, since the default of `slide` is stated to be `Yes.withPartial`, shouldn't `[["1", "2"], ["2"]]` be the correct answer, since the last element has less than two entries?It seems withPartial thing should work only when the specific Window specified. Because in other case the whole range is available.
Jun 16 2023
On Fri, Jun 16, 2023 at 08:41:48AM +0000, FeepingCreature via Digitalmars-d wrote:On Thursday, 15 June 2023 at 19:18:03 UTC, H. S. Teoh wrote:[...]https://issues.dlang.org/show_bug.cgi?id=23976I actually need some help with this. The behavior of `withPartial` seems to be random! For instance, since the default of `slide` is stated to be `Yes.withPartial`, shouldn't `[["1", "2"], ["2"]]` be the correct answer, since the last element has less than two entries?[...] OK, sorry for the really slow response, but I finally took a look at the current Phobos code and the implementation of withPartial. I think I've figured out how the original design works. Looks like the original design was that the window (of size windowSize) is advanced by stepSize every iteration, stopping *once the last element of the original range has been reached*. The withPartial parameter appears to be meaningful only for the case where stepSize > 1, because that's the only time you'll get a window of size < windowSize. I.e., the way I understand the original, based on my reading of the code, is this: let's say we have a range like this: 0 1 2 3 4 5 6 then for windowSize=2, stepSize=1, we have the following windows: 0 1 2 3 4 5 6 [ ] [ ] [ ] [ ] [ ] [ ] At this point, the window is [5, 6], which means the last element 6 of the original range has been reached, so the range stops. But with stepSize=2: 0 1 2 3 4 5 6 [ ] [ ] [ ] [] Here, after [4, 5] we haven't reached the last element 6 yet, so we have to advance the window once more. But there are no more elements after 6, so the next window contains only [6]. This is where withPartial comes into effect: if it's Yes, which is the default (sorry, I was wrong about this in my OP), then [6], which is smaller than windowSize, will be included in the output. Otherwise, it will be discarded. In other words, the stopping criteria is whether the last element has been reached yet (and not necessarily at the start of a window). When stepSize=1, this will always produce a full-sized window as the last element, because once that last full-sized window is generated, the last element of the original range has been seen, and the range will end. The only time you'll get a window of less the full size is when stepSize > 1. Another example: if windowSize=2, stepSize=3: 0 1 2 3 4 5 6 7 8 [ ] [ ] [ ] After the window [6,7], advancing the window by stepSize=3 passes the end of the original range, so no more windows are produced. Hope this helps clear up the intended semantics of slide(). We should probably also document the above behaviour, because the current docs are not clear at all about exactly how this works. T -- The peace of mind---from knowing that viruses which exploit Microsoft system vulnerabilities cannot touch Linux---is priceless. -- Frustrated system administrator.
Jun 21 2023
On Wednesday, 21 June 2023 at 16:49:00 UTC, H. S. Teoh wrote:[snip] Hope this helps clear up the intended semantics of slide(). We should probably also document the above behaviour, because the current docs are not clear at all about exactly how this works. TYou can go pretty far with unittest examples. ```d /// Example: windowSize=2, stepSize=1 unittest { import std.algorithm.comparison : equal; import std.range: iota, slide; auto x = 7.iota.slide(2, 1); /** Results in the following windows: 0 1 2 3 4 5 6 [ ] [ ] [ ] [ ] [ ] [ ] */ assert(x.equal!equal( [[0, 1], [1, 2], [2, 3], [3, 4], [4, 5], [5, 6]])); } /// Example: windowSize=2, stepSize=2 unittest { import std.algorithm.comparison : equal; import std.typecons: No; import std.range: iota, slide; auto x = 7.iota.slide(2, 2); /** Results in the following windows: 0 1 2 3 4 5 6 [ ] [ ] [ ] [] */ assert(x.equal!equal( [[0, 1], [2, 3], [4, 5], [6]])); // If prefer to drop the partial window, set `f` to `No.withPartial` auto y = 7.iota.slide!(No.withPartial)(2, 2); /** Results in the following windows: 0 1 2 3 4 5 6 [ ] [ ] [ ] */ assert(y.equal!equal( [[0, 1], [2, 3], [4, 5]])); } /// Example: windowSize=2, stepSize=3 unittest { import std.algorithm.comparison : equal; import std.range: iota, slide; auto x = 9.iota.slide(2, 3); /** Results in the following windows: 0 1 2 3 4 5 6 7 8 [ ] [ ] [ ] */ assert(x.equal!equal( [[0, 1], [3, 4], [6, 7]])); } ```
Jun 21 2023
On Wed, Jun 21, 2023 at 05:34:53PM +0000, jmh530 via Digitalmars-d wrote:On Wednesday, 21 June 2023 at 16:49:00 UTC, H. S. Teoh wrote:[...][snip] Hope this helps clear up the intended semantics of slide(). We should probably also document the above behaviour, because the current docs are not clear at all about exactly how this works.You can go pretty far with unittest examples.[...] Awesome, we should include these in the PR! T -- Designer clothes: how to cover less by paying more.
Jun 21 2023