Skip to content

Fix duplicating each entry if a list of bools is returned#48

Open
mgotz wants to merge 2 commits intoklemenv:mainfrom
hz-b:bool_array_fix
Open

Fix duplicating each entry if a list of bools is returned#48
mgotz wants to merge 2 commits intoklemenv:mainfrom
hz-b:bool_array_fix

Conversation

@mgotz
Copy link
Contributor

@mgotz mgotz commented Mar 3, 2026

I noticed a bug, when returning a list of bools from python. Each entry would apear doubly in the EPICS waveform.

Take this database:

record(waveform, "bools") {
    field(DTYP, "pydev")
    field(FTVL, "CHAR")
    field(NELM, 10)
    field(PINI, "YES")
    field(INP, "@[True, False]*5")
}

The waveform should contain: 1,0,1,0,1,0,1,0,1,0. Instead it contains: 1,1,0,0,1,1,0,0,1,1

Python bools are implemented as integers.
Thus in pywrapper.cpp conversion the PyLong_Check matches and the PyBool_Check as well and each entry is pushed to val twice.
Since Bools are integers under the hood anyhow, I think there is no need for an extra case.

PyBool is implemented as a subclass of int, thus bools were added twice to the val.
Once from PyLong_Check and once from PyBool_Check.
Treating bools separately is unnecessary because they are treated fine as long.
@klemenv
Copy link
Owner

klemenv commented Mar 3, 2026

Good catch. There's actually a possibility for a similar issues between long vs int in Python2.

Could you change your patch to instead move the PyBool_Check before the PyIn_Check and then add ''continue" into every " if" block in the vector for loop.

@mgotz
Copy link
Contributor Author

mgotz commented Mar 4, 2026

I changed the type checks inside the vector loop to else if statements. To me this is the much clearer construct than using if with continue.
If you want I'll readd the PyBool_Check, but it is redundant. In Python2 Bools are also implemented as integers and all we need here is to have an integer value for the bool.

@klemenv
Copy link
Owner

klemenv commented Mar 8, 2026

Let's stick with 'continue'. I prefer 'continue', 'break', 'return' in long code blocks like this one because when reading the code, upon encountering 'continue' one can be ensured that even if there was any other code after if...else if...else, it would be skipped. In a sense it keeps readers' focus on the code block at hand only, instead of scrolling up and down a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants