Change in pysim[master]: Add more type annotations

fixeria gerrit-no-reply at
Sat Apr 3 23:33:22 UTC 2021

fixeria has posted comments on this change. ( )

Change subject: Add more type annotations

Patch Set 2:


Some comments are not related to your change, but rather express my ideas on potential improvements. I could not resist while reading the code. 
File pySim/ 
PS2, Line 31: str
"a string of hex nibbles" - Hexstr? 
PS2, Line 39: str
"a string of hex nibbles" - Hexstr? 
PS2, Line 54: # List of bytes to string
unrelated: this comment is out of sync with the docstring below:

  "List of bytes" vs "list of integers" 
PS2, Line 113: return None
A proper Pythonic approach would be to raise exceptions here... So either it returns something, or it fails. Also unrelated, of course. 
PS2, Line 157: plmn
PS2, Line 166: plmn
PS2, Line 192: fivehexbytes:Hexstr
I still find it odd that we (ab)use strings to represent bytes in a language where we have a special type for that... 
PS2, Line 278: ->int
cosmetic: missing space 
PS2, Line 290: 	if imsi == None:
Your type hint renders this check useless. I would remove it because it's wrong to pass None and expect this function to decode something. Looks like over-defensive programming to me. 
PS2, Line 298: long
PS2, Line 302: 	if imsi == None:
Same here. 
PS2, Line 350: ef_msisdn
PS2, Line 371: return None
I think we should rather 'raise ValueError' here and drop the 'Optional' from the returned value. Mixing a value, None, and exceptions is one of the worst things that I saw... Just an idea for a separate patch. 
PS2, Line 417: st
Hexstr? (not sure here, some magic happens on line 432)

To view, visit
To unsubscribe, or for help writing mail filters, visit

Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I50a0a07132890af0817f4ff0ce9fec53b7512522
Gerrit-Change-Number: 23594
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge at>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at>
Gerrit-Reviewer: fixeria <vyanitskiy at>
Gerrit-Comment-Date: Sat, 03 Apr 2021 23:33:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <>

More information about the gerrit-log mailing list