Skip to content

Fix variable array write semantics and VariableWait state handling#1144

Merged
bengineerd merged 4 commits into
pre-releasefrom
bugfix/variable-array-wait-semantics
Mar 26, 2026
Merged

Fix variable array write semantics and VariableWait state handling#1144
bengineerd merged 4 commits into
pre-releasefrom
bugfix/variable-array-wait-semantics

Conversation

@bengineerd

@bengineerd bengineerd commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

Bug Fixes

  1. BaseVariable _setDict() failed on single-index array keys

    What was broken:

    • Single-index expressions like "1" were evaluated to scalar 1, but the code assumed a subscriptable result and indexed [0].

    Inputs that triggered it:

    • Array-element config updates through _setDict().

    Example:

    • root.CmdDev.RemoteArray._setDict("99", writeEach=False, modes=["RW"], keys=["1"])

    Previous failure:

    • TypeError: 'int' object is not subscriptable

    Fix:

    • Handled scalar index results and slice/list results separately.
  2. Indexed setDisp() on ndarray-backed variables passed a 0-D ndarray instead of a scalar

    What was broken:

    • parseDisp("99") for ndarray-backed variables can produce a 0-D ndarray.
    • Indexed writes need a scalar element.

    Inputs that triggered it:

    • Indexed display-string writes into ndarray-backed variables.

    Example:

    • root.CmdDev.RemoteArray.setDisp("99", index=1)

    Result:

    • Invalid ndarray-dimension/type failure during the element write path.

    Fix:

    • Unwrapped 0-D ndarrays with .item() before indexed set() calls.
  3. VariableWaitClass.get_values() used variable objects instead of stored path keys

    What was broken:

    • Internal state is keyed by v.path.
    • get_values() tried to fetch entries using the variable objects themselves.

    Inputs that triggered it:

    • Calling get_values() after VariableWaitClass.wait().

    Example:

    • waiter = VariableWaitClass(root.Dev.Counter, timeout=...)
    • waiter.wait()
    • waiter.get_values()

    Fix:

    • Returned {variable_object: cached_value_by_path} using k.path internally.
  4. Documented VariableValue.updated field did not actually exist

    What was broken:

    • VariableWait docs describe varValues[i].updated.
    • VariableValue had no such field, and wait updates never set it.

    Inputs that triggered it:

    • Any VariableWait predicate using update-state semantics rather than raw value comparisons.

    Example:

    • VariableWait([var1, var2], lambda vals: vals[0].updated and vals[1].updated, timeout=...)

    Fix:

    • Added updated = False to VariableValue
    • Set it False on arm
    • Set it True in VariableWaitClass._varUpdate()
  5. Variable-array YAML slices did not support scalar broadcast

    What was broken:

    • Variable-level YAML array selectors already supported:
      • SomeArrayValue[*]: value
      • SomeArrayValue[:]: value
    • But slice selectors like:
      • SomeArrayValue[1:3]: value
        did not support a scalar value.
    • The implementation assumed slice assignments were always iterable and tried to iterate the scalar.

    Inputs that triggered it:

    • Any YAML load or setYaml() call that assigned a scalar to an array-variable slice.

    Example failure:

    • YAML like:
      • SomeArrayValue[1:3]: 7
    • raised:
      • TypeError: 'int' object is not iterable

    Fix:

    • Updated BaseVariable._setDict() so a single scalar value broadcasts across the selected slice.
    • Preserved support for iterable and comma-separated multi-value slice assignments.

This commit updates the BaseVariable class to ensure that indexed writes into ndarray-backed variables correctly handle scalar elements. It modifies the value assignment logic to convert 0-D ndarrays to scalars when necessary, improving data integrity during variable updates. Additionally, it refines index handling for single entry items to support both list and non-list index types.

This bug was found by the new test harness.
This commit modifies the VariableWaitClass to initialize the 'updated' state of variable values to False upon addition, and updates the state to True when a variable is modified. Additionally, the VariableValue class is updated to include an 'updated' attribute, enhancing the tracking of variable changes within the PyRogue framework.
@slacrherbst

Copy link
Copy Markdown
Contributor

This all looks ok. Only concern might be the number of ifs added and impact on performance.

@bengineerd bengineerd merged commit 681010b into pre-release Mar 26, 2026
6 checks passed
@bengineerd bengineerd deleted the bugfix/variable-array-wait-semantics branch March 26, 2026 21:24
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 13.63636% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.51%. Comparing base (80558d1) to head (0d1933f).
⚠️ Report is 5 commits behind head on pre-release.

Files with missing lines Patch % Lines
python/pyrogue/_Variable.py 13.63% 18 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@               Coverage Diff               @@
##           pre-release    #1144      +/-   ##
===============================================
- Coverage        32.55%   32.51%   -0.05%     
===============================================
  Files               68       68              
  Lines             7147     7163      +16     
  Branches          1009     1014       +5     
===============================================
+ Hits              2327     2329       +2     
- Misses            4655     4668      +13     
- Partials           165      166       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

4 participants