Change in pysim[master]: utils: Introduce DataObject representation

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

laforge gerrit-no-reply at lists.osmocom.org
Mon May 3 12:18:07 UTC 2021


laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/24035 )

Change subject: utils: Introduce DataObject representation
......................................................................


Patch Set 2:

(6 comments)

https://gerrit.osmocom.org/c/pysim/+/24035/2/pySim/utils.py 
File pySim/utils.py:

https://gerrit.osmocom.org/c/pysim/+/24035/2/pySim/utils.py@869 
PS2, Line 869: DataObject
> Is this an abstract class?
I still have my troubles understanding the 'abc' documentation. I read it several times during past mongs, to no help. It seems to discuss all kinds of things we're not really doing here and uses al ot of language that's hard to understand for me.

Your recent examples look super simlpe in terms of what they do to the code, but I somehow am not certain enough that we're using it like it's documented.

So to answer your question: I have no fxcing clue if what I'm doing here falls within the documented use cases of an abc.ABC, sorry.


https://gerrit.osmocom.org/c/pysim/+/24035/2/pySim/utils.py@873 
PS2, Line 873: this
> cosmetic: missing dot
Ack


https://gerrit.osmocom.org/c/pysim/+/24035/2/pySim/utils.py@884 
PS2, Line 884:         self.encoded = None
> One of these should be 'decoded' I guess, and this is why pylint complains.
Ack


https://gerrit.osmocom.org/c/pysim/+/24035/2/pySim/utils.py@972 
PS2, Line 972: from_value
> Sounds more like from_bytes() given that it accepts 'bytes'?
Ack


https://gerrit.osmocom.org/c/pysim/+/24035/2/pySim/utils.py@985 
PS2, Line 985: members=[]
> Using objects, lists, and tuples as a default argument value is dangerous because all instances of t […]
Ah yes, I read about this some time ago, but forgot it. 

How would you work around this here? I don't want to manually call the 'add' operator here inf my use cases (see following patch) where I want a compact syntax to intiialize a collection/choice with various members.


https://gerrit.osmocom.org/c/pysim/+/24035/2/pySim/utils.py@991 
PS2, Line 991:         for m in self.members:
> This could be done using the comprehension: […]
but how would I select the m.tag / m.name here? 

 self.members_by_tag = { m.tag:m for m in members }



-- 
To view, visit https://gerrit.osmocom.org/c/pysim/+/24035
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Iac18e7665481c9323cc7d22a3cd93e3da7869deb
Gerrit-Change-Number: 24035
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Comment-Date: Mon, 03 May 2021 12:18:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210503/25042ee7/attachment.htm>


More information about the gerrit-log mailing list