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 <nhofm...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria <vyanits...@sysmocom.de>
Gerrit-CC: laforge <lafo...@osmocom.org>
Gerrit-Attention: laforge <lafo...@osmocom.org>
Gerrit-Attention: fixeria <vyanits...@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 <lafo...@osmocom.org>
