py/objint: Make byteorder argument optional in byte-conversion methods.

[AmirHmZz]

This PR intends to make byteorder argument optional in int.to_bytes() and int.from_bytes() methods. According to CPython documentation, byteorder argument is optional and set to "big" by default. This PR makes Micropython compatible with CPython in this case.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.39%. Comparing base (e60e807) to head (f8961ea).
Report is 1 commits behind head on master.

❗ Current head f8961ea differs from pull request most recent head ca84705. Consider uploading reports for the commit ca84705 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #14387   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files         161      161           
  Lines       21204    21204           
=======================================
  Hits        20864    20864           
  Misses        340      340           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Code size report:

   bare-arm:   +20 +0.035% 
minimal x86:   +48 +0.026% 
   unix x64:   +40 +0.005% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +8 +0.002% RPI_PICO
       samd:    +8 +0.003% ADAFRUIT_ITSYBITSY_M4_EXPRESS

[projectgus]

Hi @AmirHmZz,

Thanks for noticing this and submitting this PR.

According to CPython documentation, byteorder argument is optional and set to "big" by default. This PR makes Micropython compatible with CPython in this case.

To expand a bit on this, these byteorder arguments only became optional in CPython 3.11 (3.10 version is here). MicroPython only advertises full compatibility with CPython 3.4 at this point.

That said, there are a number of improvements from later CPython versions that are supported in MicroPython and this one might be useful to have. 😁

I was going to suggest adding unit test cases for these as well, although it will be a little fiddly because the unit tests compare behaviour with CPython by default, and only some CPython versions will accept the optional argument.

[AmirHmZz]

@projectgus One of the most important points about making that arg optional is reducing redundant allocations and deallocations on "big" string which is used in most cases. I use int>bytes and bytes>int conversions many many times in my projects and most of them are in the loops.

I was going to suggest adding unit test cases for these as well, although it will be a little fiddly because the unit tests compare behaviour with CPython by default, and only some CPython versions will accept the optional argument.

Where should I add unit tests? I will be glad to contribute!

[projectgus]

I asked @dpgeorge about this change and it seems like it really will depend on the code size hit as to whether it's desirable to merge. It's currently small, which is encouraging.

However, Damien noted that in Python 3.11 the int.to_bytes() function also gained a default value for the length argument. What's the size impact if this is added as well? It's desirable to keep the CPython compatibility changes consistent as possible, although if that additional change somehow has much higher code size impact then it can be dropped.

Where should I add unit tests? I will be glad to contribute!

Thanks, that'd be great!

tests/basic/int_bytes.py has the current test cases for int.to_bytes and int.from_bytes. However, this test file works by comparing output to CPython which won't be consistent for this change. You'll need to do something like tests/basics/string_format_cp310.py and tests/basics/string_format_cp310.exp which has the test case and expected output in separate files.

Suggest something like int_bytes_optional_args_cp311.py.

making that arg optional is reducing redundant allocations and deallocations on "big" string which is used in most cases. I use int>bytes and bytes>int conversions many many times in my projects and most of them are in the loops.

I would have expected if you're either freezing your .py files into the firmware, or compiling to .mpy with mpy-cross, then the "big" string should end up interned as a QSTR with minimal performance impact. If you've got some "real world" type code where this change makes a significant performance difference, please share it as it'd be interesting to look into.

[AmirHmZz]

@projectgus Thanks for feedback. I've added tests and also made length optional in int.to_bytes().

[projectgus]

Nice work @AmirHmZz, thanks! Implementation and tests look good to me. It's encouraging this implementation is free or almost free in terms of code size on most of the smaller microcontroller ports.

[projectgus]

Looks good to me, I think whether this is merged will come down to @dpgeorge's call about the binary size cost.

[projectgus]

One thing about the optional length argument for int.to_bytes(), currently MicroPython will truncate to the length if the int doesn't fit (CPython raises an exception.) Which may be a bit counter-intuitive. However, I have a PR open (#13087) which changes the behaviour to match CPython on this.

(Regrettably, that PR adds more than 8 bytes binary size! 😆)