Attention is currently required from: fixeria, laforge. neels has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/41918?usp=email )
Change subject: ConfigurableParameter: do not magically overwrite the 'name' attribute ...................................................................... Patch Set 6: (1 comment) Commit Message: https://gerrit.osmocom.org/c/pysim/+/41918/comment/bb8982c7_7651b9ae?usp=email : PS2, Line 7: Co > > This tiny detail is just not worth even considering to spend time on. […] I apologized out-of-band for the emotional outburst, let me repeat this here: sorry for the rant. The automatic naming has the problem that the names have to be valid python class names. For example, "5GS-SUCI-CalcInfo" would not work as a name. You are right, we can check in __new__() whether it is already set and only change it otherwise. Indeed that is an ok solution with even a slight advantage (in the mix of disadvantages), over my different solution, using the get_name() classmethod. But: * It is not necessary to create automatic names. I would rather set manual names explicitly on every subclass. I have gotten code review that the automatic naming should remain. So I've put it back in the form of the get_name() classmethod, using normal python inheritance. * It is a bad idea to do anything hidden in a __new__() of a superclass, because "explicit is better than implicit" https://peps.python.org/pep-0020/ and it makes it hard to adjust subclass behavior in the usual pythonic inheritance. This is my opinion and you do not have to agree, even though I know I'm right =) * My get_name() classmethod implementation has a problem that I don't like very much: callers have to call get_name() to get the automatic name. They could also just access `foo.name`, but then they will get only a manually set name. So callers have to be aware of that, which is not very good API design from my side. This is ofc not a problem whenever a name was set manually. * When all names are set manually anyway, which is the case in my patches, the discussion has very low to actually zero relevance to the real world. That is why my opinion is that my colleagues should be aware of that, and simply trust my judgement (if i am wrong it would have no actual effect, either), so that we can work with the actually important parts of the code. This has not happened for something like a year, we are still discussing this topic of zero real world effect, and comments here even sounded to me like i were the one stopping the progress, which I would not accept to be accurate. That is why my patience has been inapproriately low on this particular aspect. Sorry about that, and I greatly appreciate that you are here to resolve this. Back to the conclusion, I am fine with either * removing all automatic names * or, if you really have to, go on and put a new decision in __new__() and remove the get_name() classmethod. I can then adjust all callers on my side later. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/41918?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: I6f631444c6addeb7ccc5f6c55b9be3dc83409169 Gerrit-Change-Number: 41918 Gerrit-PatchSet: 6 Gerrit-Owner: neels <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <[email protected]> Gerrit-CC: laforge <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Comment-Date: Tue, 05 May 2026 17:30:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <[email protected]> Comment-In-Reply-To: laforge <[email protected]> Comment-In-Reply-To: fixeria <[email protected]>
