Attention is currently required from: fixeria, laforge.
neels has posted comments on this change by neels. (
https://gerrit.osmocom.org/c/pysim/+/39741?usp=email )
Change subject: [1/6] personalization: refactor: drop ClassVarMeta use
......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS6:
I'm not entirely sure why the new approach is
considered better or an advantage over the old one. […]
Name:
Ah, I think this is an artifact from polishing and rebasing the commits on my branch. It
seems this patch is removing the 'name', and then 'name' is added back in
a commit still waiting on my branch. The intention was to have a slight tweak of
'name', all in one commit: to keep the autmatic name as it is now, but also have
the option of using an explicit string name in a subclass, so we're not limited to py
syntax. 'name' will definitely stay!
About why the branch is even touching the name part: I was at first working with the class
names themselves (the automatic 'name'), but figured that the typical format is,
for example, "PIN1", not "Pin1". For the SdKey subclasses, we probably
will want names that are more complex than python class name syntax permits. We might also
want to rename class K to a more normal class syntax like AkaKey? Anyway IMHO it is better
to keep the personalization API+UI namespace separate from python code internals.
CSV matching: I do use the 'name' in our web platform code to match CSV columns to
parameter names -- but only to aid the user with picking which column to feed to which
parameter. The CSV column matching code so far is intended to not migrate to the pysim
repos; pysim will have all the code for feeding explicitly selected CSV columns to
explicitly selected parameters, only. But I guess we could also move that automatic column
matching code here to pysim.git in some way or other. (For that matching, it does not
matter if the name is "Pin1" or "PIN1" or even "~pin: 1~",
it is comparing only lowercased and sanitized to [a-z0-9])
"Approach": you mean using ClassVarMeta is better than plain standard python? If
yes then I disagree, otherwise I am not comprehending and please elaborate.
My opinion against ClassVarMeta is that future patches add more members, and I do not
think it is a good idea to add all those as kwargs fed to a messed-with `__new__()`
builtin. Plain class members work just as well AFAICT, so i do want to stick with plain
python and proper inheritance and normal access to class members where one is initialized
from the value of the other like
~~~
class Pin(ConfigurableParameter):
min_len = 4
default_value = '0' * min_len
~~~
--
To view, visit
https://gerrit.osmocom.org/c/pysim/+/39741?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: I60ea8fd11fb438ec90ddb08b17b658cbb789c051
Gerrit-Change-Number: 39741
Gerrit-PatchSet: 6
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 13 Mar 2025 21:52:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>