tobinarray end parameter is inconsistent with standard python range behaviour

[sneakypete]

Python "range"-style functions define the stop parameter as "non-inclusive":
range(0, 10)
returns [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]

len(range(0, 10))
returns 10

However tobinarray's end parameter is "inclusive"
len(ih.tobinarray(0, 10))
returns 11

[Alexander Belchenko (bialix)]

Yes, this is intended behavior from the day one of intelhex library development. Change this is too late, because it will be serious API break.

I can only improve the documentation for `end' parameter of various functions and explicitly say it's inclusive.

[sneakypete (sneakypete81)]

Understood - and thanks for your quick response. Overall this is a fantastic module, and has proved very useful for me.

I only raise this because any deviation from python "norms" is a bug waiting to happen (fortunately it caused an exception later in my code, so I was able to catch it)

To give a quick example:
    for addr in range(0, EEPROM_SIZE, BLOCK_SIZE):
        eeprom.i2c_write(addr, ih.tobinarray(addr, addr + BLOCK_SIZE - 1))

The "-1" is messy, and shouldn't be required (and looks a lot like visual basic!).

Another alternative is to create a wrapper function with "pythonic" behaviour and deprecate the old function, so existing code is not broken but new code is more intuitive.

def to_binary_array(self, start=None, end=None, pad=None):
    if end is not None:
        end = end - 1
    self.tobinarray(start, end, pad)

[Alexander Belchenko (bialix)]

sneakypete пишет:
> Understood - and thanks for your quick response. Overall this is a
> fantastic module, and has proved very useful for me.

Thanks :-)

> I only raise this because any deviation from python "norms" is a bug
> waiting to happen (fortunately it caused an exception later in my code,
> so I was able to catch it)

Well, when I wrote that code I was more C-guy than Python-guy, may be it
explains something about something. Although I'm still think the current
behavior has some pros.

> To give a quick example:
> for addr in range(0, EEPROM_SIZE, BLOCK_SIZE):
> eeprom.i2c_write(addr, ih.tobinarray(addr, addr + BLOCK_SIZE - 1))
>
> The "-1" is messy, and shouldn't be required (and looks a lot like
> visual basic!).

Well, your code could be even better this way:

      for addr in range(0, EEPROM_SIZE, BLOCK_SIZE):
          eeprom.i2c_write(addr, ih.tobinarray(start=addr,
                                               size=BLOCK_SIZE))

Adding `size' parameter is much simpler solution.

> Another alternative is to create a wrapper function with "pythonic"
> behaviour and deprecate the old function, so existing code is not broken
> but new code is more intuitive.

You can add such wrapper yourself in your code, simply by subclassing
IntelHex. E.g.:

class IntelHexEx(IntelHex):

    def to_binary_array(self, start=None, end=None, pad=None):
        if end is not None:
            end = end - 1
        self.tobinarray(start, end, pad)

That's all :-)

[Alexander Belchenko (bialix)]

So, actually you have a workaround right now: use slice methods to get part of IntelHex object:

    for addr in range(0, EEPROM_SIZE, BLOCK_SIZE):
        eeprom.i2c_write(addr, ih[addr:addr + BLOCK_SIZE].tobinarray())

[Alexander Belchenko (bialix)]

So, I've extracted feature request for new size parameter: https://bugs.launchpad.net/intelhex/+bug/408748

This could be done easily and won't require API break.

[Alexander Belchenko (bialix)]

Requested change of end parameter behavior require API break, therefore it will require major version bump. Target this feature request to future 2.0 version.

[sneakypete (sneakypete81)]

Sounds like a good resolution - thanks Alexander.

[Alexander Belchenko (bialix)]

I'm about to release 2.0, and return again to this issue.

Standard range function has 3 parameters: start, stop [,step]
here: stop is exclusive

My tobinXXX group of methods first parameters are: start, end
here: is inclusive

I have 2 choices:
1) remove end and add stop
2) rename end to end_inclusive, and add stop additionaly.

[Alexander Belchenko (bialix)]

Moved to github https://github.com/bialix/intelhex/issues/20