Perform boolean serialization before integer serialization.

[dthiele]

Hi,

as a bool is an instance of int in Python (cf. https://peps.python.org/pep-0285/#specification), this PR changes the native type serialization order of YangDataSerialiser.default() so that bool is serialized before int. Otherwise, bools will be serialized as 0 or 1.

Best regards,
Daniel

[dthiele]

The failed "black checks" from tox -e black also fail on a plain master clone.

[JoseIgnacioTamayo]

The failed "black checks" from tox -e black also fail on a plain master clone.

#369 would fix those

[JoseIgnacioTamayo]

As there is no change in the unittests, I wonder if this PR has actually any effects, or changes anything.

@dthiele , could you please add a test under tests/serialize that shows the effect of this change?

[dthiele]

I stumbled across this issue, when I accidentally passed a plain dict (as returned by Python's json.load()) into pybindJSON.dumps(). I'm not sure if this is one of the intended use cases? If it is, I will, of course, write a corresponding test case.

[JoseIgnacioTamayo]

I stumbled across this issue, when I accidentally passed a plain dict (as returned by Python's json.load()) into pybindJSON.dumps(). I'm not sure if this is one of the intended use cases? If it is, I will, of course, write a corresponding test case.

Looking at the unit test code in https://github.com/robshakir/pyangbind/tree/master/tests/serialise and in my experience using pyangbind I would say no, the sequence or steps you performed is not the intended use.

I would suggest you raise an Issue linked to this PR. If other people upvote or comment there, it means it is a common use and this fix is needed.

[dthiele]

OK, I found a case (leaf-list of boolean), which triggers the serialization issue and added a corresponding test case. Could you take a look?

Note that a convenient comparison via == on Python dicts, e.g. as performed here https://github.com/dthiele/pyangbind/blob/fix-serialization-order-0/tests/serialise/json-serialise/run.py#L73, won't do , as Python (per https://peps.python.org/pep-0285/#specification) also does not differentiate boolean and int when comparing with == (this is why I had to use assertIs() in the new test case). This also implies that we may not rely on dict comparison for serialization tests. E.g. if you change "boolean": true to "boolean": 1 in https://github.com/robshakir/pyangbind/blob/master/tests/serialise/json-serialise/json/expected-output.json#L53, the test still succeeds. Shall I open an issue for this?

[JoseIgnacioTamayo]

Thanks @dthiele for checking it.

Indeed that sounds like an issue, although practically I guess most users do use == to compare the boolean values.

We can link the issue to this PR, which helps track what problem is this PR solving.

[dthiele]

I have created #370 regarding the test cases.

For the rest of the discussion, I would like to limit the focus on the PR (sorry for diverging before):

• This PR addresses a problem with pyangbind's JSON serialization.
• A new test case has been provided to show this.
• A fix is provided in this PR.

Could you take a look if this can be merged or if there is anything missing?

Best regards,
Daniel