Attention is currently required from: dexter, laforge, neels.

fixeria has posted comments on this change by neels. ( 
https://gerrit.osmocom.org/c/pysim/+/39742?usp=email )

Change subject: personalization: refactor ConfigurableParameter, Iccid, Imsi
......................................................................


Patch Set 15:

(1 comment)

File pySim/esim/saip/personalization.py:

https://gerrit.osmocom.org/c/pysim/+/39742/comment/8185ff57_1a27e96d?usp=email :
PS13, Line 50:     r"""Base class representing a part of the eSIM profile that 
is configurable during the
> abc has no purpose and it doesn't work.


This is quite a bold statement. I don't know what you expect from it, but in my 
experience it does the job: a) classes that inherit from `abc.ABC` (and their 
sub-classes that inherit indirectly) cannot be instantiated unless all the 
abstract methods are defined; b) it clearly tells to whoever is reading the 
code that it's an abstract class.


I must admit Python's abstract class model is not as advanced as in other 
languages, like Java or C#. For instance, you can still inherit a class without 
defining all abstract classmethods in it - this will fail not during parsing, 
but during instantiation at run-time. Yet it's still better than nothing.

Harald's concern is perfectly valid: using `abc.abstractmethod` but not 
inheriting from `abc.ABC` will result in the former doing nothing. The ABC 
magic simply does not work without that.

BTW, the build verification failure is a good argument for using abc stuff:

https://jenkins.osmocom.org/jenkins/job/gerrit-pysim-build/JOB_TYPE=test,a1=default,a3=default,a4=default,label=simtester/2680/consoleFull

```
TypeError: Can't instantiate abstract class Puk1 with abstract method apply_val

```

Looks like there's some mix-up of `apply()` and `apply_val()`?



--
To view, visit https://gerrit.osmocom.org/c/pysim/+/39742?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I6522be4c463e34897ca9bff2309b3706a88b3ce8
Gerrit-Change-Number: 39742
Gerrit-PatchSet: 15
Gerrit-Owner: neels <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: neels <[email protected]>
Gerrit-CC: fixeria <[email protected]>
Gerrit-Attention: neels <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Attention: dexter <[email protected]>
Gerrit-Comment-Date: Sun, 11 Jan 2026 22:54:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <[email protected]>
Comment-In-Reply-To: laforge <[email protected]>

Reply via email to