From e82680cf5c5d10e821384a21ce97ce557989b91e Mon Sep 17 00:00:00 2001 From: Milad Nasrollahi Date: Wed, 21 Apr 2021 23:21:27 -0700 Subject: [PATCH 01/11] Allow count to accept floats, nan, inf and remove unused struct value The previous iteration of count only handled integer values. An attempt is made here to allow float inputs alongside integer ones. That means count(0.5, 3), count(float('nan'), 2), etc are now valid inputs and should yield outputs similar to Python's itertools.count. A value of the struct countObject is also removed since it was doing nothing. --- itertools.go | 161 +++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 125 insertions(+), 36 deletions(-) diff --git a/itertools.go b/itertools.go index bc47d65..5ba29a5 100644 --- a/itertools.go +++ b/itertools.go @@ -6,24 +6,115 @@ import ( "go.starlark.net/starlark" ) -type countObject struct { - cnt int - step int - frozen bool - value starlark.Value +// Type that attempts to allow operations between numerics, +// i.e. float and int. +type floatOrInt struct { + f_ *starlark.Float + i_ *starlark.Int +} + +// Unpacker for float or int type. This allows int types and float +// types to interact with one another, e.g. count(0, 0.1). +// type floatOrInt float6 +func (p *floatOrInt) Unpack(v starlark.Value) error { + errorMsg := "floatOrInt must have default initialization" + + switch v := v.(type) { + case starlark.Int: + if p.f_ != nil { + return fmt.Errorf(errorMsg) + } + p.i_ = &v + return nil + case starlark.Float: + if p.i_ != nil { + return fmt.Errorf(errorMsg) + } + p.f_ = &v + return nil + } + return fmt.Errorf("got %s, want float or int", v.Type()) +} + +func (fi *floatOrInt) add(n floatOrInt) error { + switch { + case fi.i_ != nil && n.i_ != nil: + x := fi.i_.Add(*n.i_) + fi.i_ = &x + return nil + case fi.i_ != nil && n.f_ != nil: + x := starlark.Float(float64(fi.i_.Float()) + float64(*n.f_)) + fi.f_ = &x + fi.i_ = nil + return nil + case fi.f_ != nil && n.i_ != nil: + x := starlark.Float(float64(*fi.f_) + float64(n.i_.Float())) + fi.f_ = &x + fi.i_ = nil + return nil + case fi.f_ != nil && n.f_ != nil: + x := starlark.Float(float64(*fi.f_) + float64(*n.f_)) + fi.f_ = &x + return nil + } + return fmt.Errorf("float to int addition not possible") } -func newCountObject(cnt int, stepValue int) *countObject { - return &countObject{cnt: cnt, step: stepValue, value: starlark.MakeInt(cnt)} +func (fi floatOrInt) string() string { + switch { + case fi.i_ != nil: + return fi.i_.String() + case fi.f_ != nil: + return fi.f_.String() + default: + // This block should not be reached. + // starlark's String() method is being replicated + // so an error is not raised. + return "" + } +} + +// Equality operator between floatOrInt and starlark's Int, Float +// and Golang's int. +func (fi *floatOrInt) eq(v interface{}) bool { + switch v := v.(type) { + case starlark.Int: + if fi.i_ != nil && *fi.i_ == v { + return true + } else { + return false + } + case starlark.Float: + if fi.f_ != nil && *fi.f_ == v { + return true + } else { + return false + } + case int: + if fi.i_ == nil { + return false + } + var x int + starlark.AsInt(*fi.i_, &x) + return x == v + } + + return false +} + +type countObject struct { + cnt floatOrInt + step floatOrInt + frozen bool } func (co *countObject) String() string { // As with the cpython implementation, we don't display - // step when it is an integer equal to 1. - if co.step == 1 { - return fmt.Sprintf("count(%v)", co.cnt) + // step when it is an integer equal to 1 (default step value). + if co.step.eq(1) { + return fmt.Sprintf("count(%v)", co.cnt.string()) } - return fmt.Sprintf("count(%v, %v)", co.cnt, co.step) + return fmt.Sprintf("count(%v, %v)", co.cnt.string(), co.step.string()) } func (co *countObject) Type() string { @@ -33,7 +124,6 @@ func (co *countObject) Type() string { func (co *countObject) Freeze() { if !co.frozen { co.frozen = true - co.value.Freeze() } } @@ -58,8 +148,16 @@ func (c *countIter) Next(p *starlark.Value) bool { if c.co.frozen { return false } - *p = starlark.MakeInt(c.co.cnt) - c.co.cnt += c.co.step + + switch { + case c.co.cnt.i_ != nil: + *p = c.co.cnt.i_ + case c.co.cnt.f_ != nil: + *p = c.co.cnt.f_ + } + + c.co.cnt.add(c.co.step) + return true } @@ -72,37 +170,28 @@ func count_( kwargs []starlark.Tuple, ) (starlark.Value, error) { var ( - start int - step int + defaultStart = starlark.MakeInt(0) + defaultStep = starlark.MakeInt(1) + start floatOrInt = floatOrInt{} + step floatOrInt = floatOrInt{} ) if err := starlark.UnpackPositionalArgs( "count", args, kwargs, 0, &start, &step, ); err != nil { return nil, fmt.Errorf( - "Got %v but expected NoneType or valid integer values for "+ - "start and step, such as (0, 1).", args.String(), + "Got %v but expected no args, or one or two valid numbers", + args.String(), ) } - const ( - defaultStart = 0 - defaultStep = 1 - ) - // The rules for populating the count object based on the number - // of args passed is as follows: - // 0 args -> default values for start and step - // 1 args -> arg defines start, default for step - // 2 args -> both start and step are defined by args - var co_ *countObject - switch nargs := len(args); { - case nargs == 0: - co_ = newCountObject(defaultStart, defaultStep) - case nargs == 1: - co_ = newCountObject(start, defaultStep) - default: // nargs == 2 - co_ = newCountObject(start, step) + // Check if start or step require default values. + if start.f_ == nil && start.i_ == nil { + start.i_ = &defaultStart + } + if step.f_ == nil && step.i_ == nil { + step.i_ = &defaultStep } - return co_, nil + return &countObject{cnt: start, step: step}, nil } From 59b18987af653699210e65b8cbc15194d0204a37 Mon Sep 17 00:00:00 2001 From: Milad Nasrollahi Date: Wed, 21 Apr 2021 23:25:01 -0700 Subject: [PATCH 02/11] Add exhaustive tests for countObject's floatOrInt input Tests for different combinations of inputs and negative values are included. --- testdata/itertools.star | 94 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 1 deletion(-) diff --git a/testdata/itertools.star b/testdata/itertools.star index e3c0a22..9020397 100644 --- a/testdata/itertools.star +++ b/testdata/itertools.star @@ -29,8 +29,97 @@ def test_count(): assert.eq(str(c2), "count(11, 3)") assert.eq(next(c2), 11) + # Negative args. + c3 = count(-5, -10) + assert.eq(str(c3), "count(-5, -10)") + assert.eq(next(c3), -5) + assert.eq(str(c3), "count(-15, -10)") + assert.eq(next(c3), -15) + + c4 = count(5, -5) + assert.eq(str(c4), "count(5, -5)") + assert.eq(next(c4), 5) + assert.eq(str(c4), "count(0, -5)") + assert.eq(next(c4), 0) + assert.eq(str(c4), "count(-5, -5)") + assert.eq(next(c4), -5) + + # Int start, float step. + c5 = count(0, 0.1) + assert.eq(str(c5), "count(0, 0.1)") + assert.eq(next(c5), 0) + assert.eq(str(c5), "count(0.1, 0.1)") + assert.eq(next(c5), 0.1) + assert.eq(str(c5), "count(0.2, 0.1)") + assert.eq(next(c5), 0.2) + + # Float start, int step — this should be handled same as above + # but check to be exhaustive. + c6 = count(0.5, 5) + assert.eq(str(c6), "count(0.5, 5)") + assert.eq(next(c6), 0.5) + assert.eq(str(c6), "count(5.5, 5)") + assert.eq(next(c6), 5.5) + assert.eq(str(c6), "count(10.5, 5)") + assert.eq(next(c6), 10.5) + + # This test may seem similar to c5 but is different because + # here step > 1. In the case that 0 < step < 1, fmt.Sprintf, + # which is used in String(), will display it as a float but + # may display it as an int if the proper flags aren't used. + c7 = count(5.0, 0.5) + assert.eq(str(c7), "count(5.0, 0.5)") + assert.eq(next(c7), 5.0) + assert.eq(str(c7), "count(5.5, 0.5)") + assert.eq(next(c7), 5.5) + assert.eq(str(c7), "count(6.0, 0.5)") + assert.eq(next(c7), 6.0) + + # NaNs + c8 = count(0, float('nan')) + assert.eq(str(c8), "count(0, %s)" % (float('nan'))) + assert.eq(next(c8), 0) + assert.eq(str(c8), "count(%s, %s)" % (float('nan'), float('nan'))) + assert.eq(next(c8), float('nan')) + assert.eq(str(c8), "count(%s, %s)" % (float('nan'), float('nan'))) + assert.eq(next(c8), float('nan')) + + c9 = count(0, float("+inf")) + assert.eq(str(c9), "count(0, %s)" % (float("+inf"))) + assert.eq(next(c9), 0) + assert.eq(str(c9), "count(%s, %s)" % (float("+inf"), float("+inf"))) + assert.eq(next(c9), float("+inf")) + assert.eq(str(c9), "count(%s, %s)" % (float("+inf"), float("+inf"))) + assert.eq(next(c9), float("+inf")) + + c10 = count(0, float("-inf")) + assert.eq(str(c10), "count(0, %s)" % (float("-inf"))) + assert.eq(next(c10), 0) + assert.eq(str(c10), "count(%s, %s)" % (float("-inf"), float("-inf"))) + assert.eq(next(c10), float("-inf")) + assert.eq(str(c10), "count(%s, %s)" % (float("-inf"), float("-inf"))) + assert.eq(next(c10), float("-inf")) + + c11 = count(float("nan"), 2) + assert.eq(str(c11), "count(%s, 2)" % (float('nan'))) + assert.eq(next(c11), float('nan')) + assert.eq(str(c11), "count(%s, 2)" % (float('nan'))) + assert.eq(next(c11), float('nan')) + + c12 = count(float("+inf"), 2) + assert.eq(str(c12), "count(%s, 2)" % (float('+inf'))) + assert.eq(next(c12), float('+inf')) + assert.eq(str(c12), "count(%s, 2)" % (float('+inf'))) + assert.eq(next(c12), float('+inf')) + + c13 = count(float("-inf"), 2) + assert.eq(str(c13), "count(%s, 2)" % (float('-inf'))) + assert.eq(next(c13), float('-inf')) + assert.eq(str(c13), "count(%s, 2)" % (float('-inf'))) + assert.eq(next(c13), float('-inf')) + # Fails - z = ("a", "b") + # Non-numeric arg fails. assert.fails( lambda: count("a", "b"), # fails uses match under the hood, which will use @@ -38,6 +127,9 @@ def test_count(): # that MatchString would accept. r'Got \(\"a\", \"b\"\)', ) + + # Too many arg fails — should be handled by UnpackArgs but + # check to be exhaustive. assert.fails( lambda: count(1, 2, 3), r'Got \(1, 2, 3\)' From 6b41ebb295c3f9cb982854bc2c670634d2736a2c Mon Sep 17 00:00:00 2001 From: Milad Nasrollahi Date: Wed, 21 Apr 2021 23:37:38 -0700 Subject: [PATCH 03/11] Fix golangci-lint unchecked error --- itertools.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/itertools.go b/itertools.go index 5ba29a5..ae3e4f0 100644 --- a/itertools.go +++ b/itertools.go @@ -95,7 +95,9 @@ func (fi *floatOrInt) eq(v interface{}) bool { return false } var x int - starlark.AsInt(*fi.i_, &x) + if e := starlark.AsInt(*fi.i_, &x); e != nil { + panic(e) + } return x == v } @@ -156,7 +158,9 @@ func (c *countIter) Next(p *starlark.Value) bool { *p = c.co.cnt.f_ } - c.co.cnt.add(c.co.step) + if e := c.co.cnt.add(c.co.step); e != nil { + panic(e) + } return true } From 8b10f964a2270572900005b1868f4f0d0df6278f Mon Sep 17 00:00:00 2001 From: Milad Nasrollahi Date: Wed, 21 Apr 2021 23:49:21 -0700 Subject: [PATCH 04/11] Add comments to add() and make error more clear --- itertools.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/itertools.go b/itertools.go index ae3e4f0..37fe277 100644 --- a/itertools.go +++ b/itertools.go @@ -38,26 +38,30 @@ func (p *floatOrInt) Unpack(v starlark.Value) error { func (fi *floatOrInt) add(n floatOrInt) error { switch { + // fi is int; n is int case fi.i_ != nil && n.i_ != nil: x := fi.i_.Add(*n.i_) fi.i_ = &x return nil + // fi is int; n is float case fi.i_ != nil && n.f_ != nil: x := starlark.Float(float64(fi.i_.Float()) + float64(*n.f_)) fi.f_ = &x fi.i_ = nil return nil + // fi is float; n is int case fi.f_ != nil && n.i_ != nil: x := starlark.Float(float64(*fi.f_) + float64(n.i_.Float())) fi.f_ = &x fi.i_ = nil return nil + // fi is float; n is float case fi.f_ != nil && n.f_ != nil: x := starlark.Float(float64(*fi.f_) + float64(*n.f_)) fi.f_ = &x return nil } - return fmt.Errorf("float to int addition not possible") + return fmt.Errorf("error with addition: types are not int, float combos") } func (fi floatOrInt) string() string { From 000de5c69a5d915b454b215229393090decedcc1 Mon Sep 17 00:00:00 2001 From: Milad Nasrollahi Date: Wed, 21 Apr 2021 23:50:54 -0700 Subject: [PATCH 05/11] Remove commented out code --- itertools.go | 1 - 1 file changed, 1 deletion(-) diff --git a/itertools.go b/itertools.go index 37fe277..2fd5fcb 100644 --- a/itertools.go +++ b/itertools.go @@ -15,7 +15,6 @@ type floatOrInt struct { // Unpacker for float or int type. This allows int types and float // types to interact with one another, e.g. count(0, 0.1). -// type floatOrInt float6 func (p *floatOrInt) Unpack(v starlark.Value) error { errorMsg := "floatOrInt must have default initialization" From e0bc4654ac6ed28e2cb2cad6bd50bcd531aba425 Mon Sep 17 00:00:00 2001 From: Milad Nasrollahi Date: Thu, 22 Apr 2021 00:13:16 -0700 Subject: [PATCH 06/11] Create clear comments for intOrFloat and remove redundant nil pointer assignment. --- itertools.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/itertools.go b/itertools.go index 2fd5fcb..67acbc9 100644 --- a/itertools.go +++ b/itertools.go @@ -8,6 +8,9 @@ import ( // Type that attempts to allow operations between numerics, // i.e. float and int. +// The relationship between starlark.Int and starlark.Float +// in floatOrInt is an XOR one. That is, if f_ points to a +// starlark.Float then i_ must be a nil pointer and vice-versa. type floatOrInt struct { f_ *starlark.Float i_ *starlark.Int @@ -35,6 +38,14 @@ func (p *floatOrInt) Unpack(v starlark.Value) error { return fmt.Errorf("got %s, want float or int", v.Type()) } +// Adding between what may be a float or an int with what also +// may be a float or an int. +// Determining which is which is done by checking whether floatOrInt's +// starlark.Int or starlark.Float pointers are nil. +// This makes assigning to floatOrInt's values dangerous: if int is +// checked first and is not nil but the value was supposed to reflect +// a float, i.e. float is also not nil, then there will probably be an +// error downstream. func (fi *floatOrInt) add(n floatOrInt) error { switch { // fi is int; n is int @@ -46,13 +57,15 @@ func (fi *floatOrInt) add(n floatOrInt) error { case fi.i_ != nil && n.f_ != nil: x := starlark.Float(float64(fi.i_.Float()) + float64(*n.f_)) fi.f_ = &x + // Note that care must be taken to erase the underlying + // starlark.Int value of the receiver since it now represents + // a starlark.Float fi.i_ = nil return nil // fi is float; n is int case fi.f_ != nil && n.i_ != nil: x := starlark.Float(float64(*fi.f_) + float64(n.i_.Float())) fi.f_ = &x - fi.i_ = nil return nil // fi is float; n is float case fi.f_ != nil && n.f_ != nil: From 27ccb4c298d76ad7b349d0eb43592c8a3b05e7a2 Mon Sep 17 00:00:00 2001 From: Milad Nasrollahi Date: Sun, 25 Apr 2021 01:02:29 -0400 Subject: [PATCH 07/11] Modify interface functions for floatOrInt to make use of interface type casting The previous iteration used a struct that had pointers to starlark.Values with a relatively complex and manual XOR relationship. This commit attempts to rectify this by making use of type casting the underlying starlark.Value to avoid potential pitfalls from using the above implementation. The function `eq` on the receiver floatOrInt is also removed since a it was only being used once in the String() method of countObject and a simple check and computation sufficed. --- itertools.go | 211 +++++++++++++++++++-------------------------------- 1 file changed, 79 insertions(+), 132 deletions(-) diff --git a/itertools.go b/itertools.go index 67acbc9..291182d 100644 --- a/itertools.go +++ b/itertools.go @@ -6,133 +6,105 @@ import ( "go.starlark.net/starlark" ) -// Type that attempts to allow operations between numerics, -// i.e. float and int. -// The relationship between starlark.Int and starlark.Float -// in floatOrInt is an XOR one. That is, if f_ points to a -// starlark.Float then i_ must be a nil pointer and vice-versa. +// float or int type to allow mixed inputs. type floatOrInt struct { - f_ *starlark.Float - i_ *starlark.Int + value starlark.Value } -// Unpacker for float or int type. This allows int types and float -// types to interact with one another, e.g. count(0, 0.1). +// Unpacker for floatOrInt. func (p *floatOrInt) Unpack(v starlark.Value) error { - errorMsg := "floatOrInt must have default initialization" - switch v := v.(type) { case starlark.Int: - if p.f_ != nil { - return fmt.Errorf(errorMsg) - } - p.i_ = &v + p.value = v return nil case starlark.Float: - if p.i_ != nil { - return fmt.Errorf(errorMsg) - } - p.f_ = &v + p.value = v return nil } return fmt.Errorf("got %s, want float or int", v.Type()) } -// Adding between what may be a float or an int with what also -// may be a float or an int. -// Determining which is which is done by checking whether floatOrInt's -// starlark.Int or starlark.Float pointers are nil. -// This makes assigning to floatOrInt's values dangerous: if int is -// checked first and is not nil but the value was supposed to reflect -// a float, i.e. float is also not nil, then there will probably be an -// error downstream. -func (fi *floatOrInt) add(n floatOrInt) error { - switch { - // fi is int; n is int - case fi.i_ != nil && n.i_ != nil: - x := fi.i_.Add(*n.i_) - fi.i_ = &x - return nil - // fi is int; n is float - case fi.i_ != nil && n.f_ != nil: - x := starlark.Float(float64(fi.i_.Float()) + float64(*n.f_)) - fi.f_ = &x - // Note that care must be taken to erase the underlying - // starlark.Int value of the receiver since it now represents - // a starlark.Float - fi.i_ = nil - return nil - // fi is float; n is int - case fi.f_ != nil && n.i_ != nil: - x := starlark.Float(float64(*fi.f_) + float64(n.i_.Float())) - fi.f_ = &x - return nil - // fi is float; n is float - case fi.f_ != nil && n.f_ != nil: - x := starlark.Float(float64(*fi.f_) + float64(*n.f_)) - fi.f_ = &x - return nil +func (f *floatOrInt) add(n floatOrInt) error { + switch _f := f.value.(type) { + case starlark.Int: + switch _n := n.value.(type) { + // int + int + case starlark.Int: + f.value = _f.Add(_n) + return nil + // int + float + case starlark.Float: + _n += _f.Float() + f.value = _n + return nil + } + case starlark.Float: + switch _n := n.value.(type) { + // float + int + case starlark.Int: + _f += _n.Float() + f.value = _f + return nil + // float + float + case starlark.Float: + _f += _n + f.value = _f + return nil + } } - return fmt.Errorf("error with addition: types are not int, float combos") + + return fmt.Errorf("cannot compute") } -func (fi floatOrInt) string() string { - switch { - case fi.i_ != nil: - return fi.i_.String() - case fi.f_ != nil: - return fi.f_.String() - default: - // This block should not be reached. - // starlark's String() method is being replicated - // so an error is not raised. - return "" - } +func (f *floatOrInt) String() string { + return f.value.String() } -// Equality operator between floatOrInt and starlark's Int, Float -// and Golang's int. -func (fi *floatOrInt) eq(v interface{}) bool { - switch v := v.(type) { - case starlark.Int: - if fi.i_ != nil && *fi.i_ == v { - return true - } else { - return false - } - case starlark.Float: - if fi.f_ != nil && *fi.f_ == v { - return true - } else { - return false - } - case int: - if fi.i_ == nil { - return false - } - var x int - if e := starlark.AsInt(*fi.i_, &x); e != nil { - panic(e) - } - return x == v +// Iterator implementation for countObject. +type countIter struct { + co *countObject +} + +func (c *countIter) Next(p *starlark.Value) bool { + if c.co.frozen { + return false + } + + *p = c.co.cnt.value + + if e := c.co.cnt.add(c.co.step); e != nil { + panic(e) } - return false + return true } +func (c *countIter) Done() {} + +// countObject implementation as a starlark.Value. type countObject struct { - cnt floatOrInt - step floatOrInt - frozen bool + cnt, step floatOrInt + frozen bool } -func (co *countObject) String() string { +func (co countObject) String() string { // As with the cpython implementation, we don't display // step when it is an integer equal to 1 (default step value). - if co.step.eq(1) { - return fmt.Sprintf("count(%v)", co.cnt.string()) + step, ok := co.step.value.(starlark.Int) + if ok { + var x int + if err := starlark.AsInt( + step, + &x, + ); err != nil { + panic(err) + } + if x == 1 { + return fmt.Sprintf("count(%v)", co.cnt.String()) + } } - return fmt.Sprintf("count(%v, %v)", co.cnt.string(), co.step.string()) + + return fmt.Sprintf("count(%v, %v)", co.cnt.String(), co.step.String()) } func (co *countObject) Type() string { @@ -158,31 +130,6 @@ func (co *countObject) Iterate() starlark.Iterator { return &countIter{co: co} } -type countIter struct { - co *countObject -} - -func (c *countIter) Next(p *starlark.Value) bool { - if c.co.frozen { - return false - } - - switch { - case c.co.cnt.i_ != nil: - *p = c.co.cnt.i_ - case c.co.cnt.f_ != nil: - *p = c.co.cnt.f_ - } - - if e := c.co.cnt.add(c.co.step); e != nil { - panic(e) - } - - return true -} - -func (c *countIter) Done() {} - func count_( thread *starlark.Thread, _ *starlark.Builtin, @@ -190,10 +137,10 @@ func count_( kwargs []starlark.Tuple, ) (starlark.Value, error) { var ( - defaultStart = starlark.MakeInt(0) - defaultStep = starlark.MakeInt(1) - start floatOrInt = floatOrInt{} - step floatOrInt = floatOrInt{} + defaultStart = starlark.MakeInt(0) + defaultStep = starlark.MakeInt(1) + start floatOrInt + step floatOrInt ) if err := starlark.UnpackPositionalArgs( @@ -206,11 +153,11 @@ func count_( } // Check if start or step require default values. - if start.f_ == nil && start.i_ == nil { - start.i_ = &defaultStart + if start.value == nil { + start.value = defaultStart } - if step.f_ == nil && step.i_ == nil { - step.i_ = &defaultStep + if step.value == nil { + step.value = defaultStep } return &countObject{cnt: start, step: step}, nil From 0eec2e66565366b2da33985a1d7f70963c009b8a Mon Sep 17 00:00:00 2001 From: Milad Nasrollahi Date: Sun, 25 Apr 2021 01:14:11 -0400 Subject: [PATCH 08/11] Fix error message in add. --- itertools.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/itertools.go b/itertools.go index 291182d..63ad16d 100644 --- a/itertools.go +++ b/itertools.go @@ -53,7 +53,7 @@ func (f *floatOrInt) add(n floatOrInt) error { } } - return fmt.Errorf("cannot compute") + return fmt.Errorf("error with addition: types are not int, float combos") } func (f *floatOrInt) String() string { From e6c97fe0e439b2558b9c7aabed673b85b8482a97 Mon Sep 17 00:00:00 2001 From: Algebra8 Date: Sun, 25 Apr 2021 09:49:48 -0500 Subject: [PATCH 09/11] Use Int64() for default comparison Co-authored-by: tdakkota --- itertools.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/itertools.go b/itertools.go index 63ad16d..8822389 100644 --- a/itertools.go +++ b/itertools.go @@ -92,15 +92,8 @@ func (co countObject) String() string { // step when it is an integer equal to 1 (default step value). step, ok := co.step.value.(starlark.Int) if ok { - var x int - if err := starlark.AsInt( - step, - &x, - ); err != nil { - panic(err) - } - if x == 1 { - return fmt.Sprintf("count(%v)", co.cnt.String()) + if x, ok := step.Int64(); ok && x == 1 { + return "count(1)" } } From e7778d87238a901b05333904f5a41114baee45c6 Mon Sep 17 00:00:00 2001 From: Milad Nasrollahi Date: Sun, 25 Apr 2021 10:55:09 -0400 Subject: [PATCH 10/11] Return false in Next() from error instead of panic --- itertools.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/itertools.go b/itertools.go index 8822389..15c78f5 100644 --- a/itertools.go +++ b/itertools.go @@ -73,7 +73,7 @@ func (c *countIter) Next(p *starlark.Value) bool { *p = c.co.cnt.value if e := c.co.cnt.add(c.co.step); e != nil { - panic(e) + return false } return true @@ -93,7 +93,7 @@ func (co countObject) String() string { step, ok := co.step.value.(starlark.Int) if ok { if x, ok := step.Int64(); ok && x == 1 { - return "count(1)" + return "count(1)" } } From b847cb7c86286ca5690bf2270d03552ff905ca40 Mon Sep 17 00:00:00 2001 From: Milad Nasrollahi Date: Mon, 26 Apr 2021 12:01:17 -0400 Subject: [PATCH 11/11] Fix default step to show variable cnt instead of 1 for String() --- itertools.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/itertools.go b/itertools.go index 15c78f5..e931cfa 100644 --- a/itertools.go +++ b/itertools.go @@ -93,7 +93,7 @@ func (co countObject) String() string { step, ok := co.step.value.(starlark.Int) if ok { if x, ok := step.Int64(); ok && x == 1 { - return "count(1)" + return fmt.Sprintf("count(%v)", co.cnt.String()) } }