Primitive types after unpack and before pack

[beraldoleal]

We know that at the very early stage of development we decided to use boot: primitive types and our types on attributes. Ex: UBInt8() and int on an attribute of a generic struct subclass.

But using the library perhaps makes more sense to hide all the UBInt* stuff. So we should define the attribute as UBInt8():

class Foo(GenericStruct):
   a = UBInt8()

But when using makes more sense to use only integers:

>>> foo = Foo()
>>> foo.a = 8
>>> foo.pack()

The same should be done on unpack, and this is the most important part. Because when unpacking we should put there a python primitive value.

>>> foo.unpack(binary_data)
>>> type(foo.a)
(int)

The trick part is that in some cases (when is not primitive) we should accept only this complex type. Ex:

class Foo(GenericStruct):
   a = Header()

foo.a should only accept (before pack) only Header types.

We realized this when creating a new napp. The napp developer should not be aware of the lib internal types, like UBInt8()

Does that makes sense ?

[cemsbr]

Yes, agreed. pack() already works like this.

[diraol]

I also agree with this behavior, I think it makes the life of those who will use the library much easier.

Maybe even for us, developers of the lib, since we will have only one behavior, not two possible ones. It makes the lib much more consistent and avoid extra code checking.

As we want to do a refactor on the library to make the unpack method not behave as an inplace method, this two changes should be considered on the same moment.

[diraol]

@beraldoleal , the issue #237 was moved to "b3". Should we move this issue to that milestone? I'm asking because it make some sense to me to solve that issue prior to this issue.

Off course that this issue can be solved before #237, but it may add some duplicated work on the future depending on the #237 solution.

[beraldoleal]

@diraol sure.

[erickvermot]

Instead of keeping a whole new class, we can use simple derivatives of native int and srt classes that implements pack, unpack, and get_size wrappers around native to_bytes and from_bytes methods.
A simple class generator UInt(nbytes) or UInt(nbits) could generate all the classes (GenericUints), greatly reducing the amount of code we have.
This will also make the use of struct module unnecessary.

[erickvermot]

@beraldoleal, @diraol, @cemsbr, @renanrodrigo
please take a look at the examples on this branch,
master...erickvermot:value_test
I think we can dramatically reduce the amount of code in the repo.

[beraldoleal]

Hi @erickvermot , thanks for taking a looking into this. My comments are below:

It looks like your branch resolves the issue, but not in a desired way. There is a lot of non-pythonic code and some inheritance good practices are being ignored.

I can tell you that, by taking a first look into this, there are things that will break and also this will require huge changes in the entire lib. This is not desired in a issue like this.

I know that this is a simple and small piece of code but everything depends on it. So, on core changes like this, talk with them @diraol and @cemsbr (I'm busy fixing some other project issues) first at IRC or in person before starting to code.

[erickvermot]

Ok then.
I do not agree this will require huge changes in the lib, but that's your call.
thanks for taking a quick look.

[beraldoleal]

No @erickvermot it is not my call. In this case, it is a team decision.

Anyway, I have changed the priority of this because of our other issues. You can play with this on the right time and absolutely that your code will not be thrown away.

So... after you talk with @diraol and @cemsbr to decide what we have to do now, please just try to put your code to work. Because I stopped looking when I saw an exception while trying to run the tests.

[erickvermot]

Ok @beraldoleal, like I said in the comment with the branch, the branch only contains a few examples to discuss de concept/idea, it was not mean to pass all the tests, I've only put about one or two hours on this.

[beraldoleal]

@erickvermot ok, no worries. Just talk to the guys before you play with this again.

[cemsbr]

@renanrodrigo had a different idea with a simpler implementation: keep .value as it is and call it when unpacking. Thus, there's no new methods to add and the user will only see primitive types after unpacking.

[erickvermot]

@cemsbr @renanrodrigo,
yes, but since there is an issue to remove .value... #237
+ this way only unpacked messages behave with primitive types...