Skip to content

Conversation

@mikeprosserni
Copy link
Contributor

What does this Pull Request accomplish?

This PR makes unpickling DigitalWaveformSignal and DigitalWaveform more robust, so that old versions can be unpickled without error. Specifically:

  • In old versions, DigitalWaveformSignal.column_index did not exist. If not provided, this defaults to signal_index.
  • In old versions, ExtendedPropertyDictionary._on_key_changed did not exist. DigitalWaveform now creates this before adding to it, if necessary.

Why should this Pull Request be merged?

Fixes bug #234

What testing has been done?

The new test___pickled_value___unpickle___is_compatible tests pass

@github-actions
Copy link
Contributor

github-actions bot commented Dec 15, 2025

Test Results

    56 files  ±0      56 suites  ±0   26m 32s ⏱️ +24s
 2 294 tests ±0   2 294 ✅ + 2  0 💤  -  2  0 ❌ ±0 
65 656 runs  ±0  65 656 ✅ +54  0 💤  - 54  0 ❌ ±0 

Results for commit f106e0d. ± Comparison against base commit 3991b93.

♻️ This comment has been updated with latest results.

@mikeprosserni mikeprosserni marked this pull request as ready for review December 15, 2025 18:54
Copy link
Collaborator

@bkeryan bkeryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged the tests PR, so you will have to resolve merge conflicts. I don't think it's critical to cherry-pick the new tests to 1.0.1.

@mikeprosserni mikeprosserni merged commit a762ef7 into main Dec 15, 2025
40 checks passed
@mikeprosserni mikeprosserni deleted the users/mprosser/bug-234-on-key-change-pickling branch December 15, 2025 20:18
mikeprosserni added a commit that referenced this pull request Dec 15, 2025
* add versioned unpickling tests for waveform and signal, and fix issues causing the new tests to fail

* skip unpickle tests in oldest_deps test run

---------

Co-authored-by: Mike Prosser <[email protected]>
mikeprosserni added a commit that referenced this pull request Dec 16, 2025
…als (#222) (#233)

* Reverse Indices for Digital Waveform Signals (#222)

* add _raw_index

* line_index and line_names

* more tests

* fix new test

* data_index and documentation

* fix doc types

* documentation improvements

* documentation improvements

* documentation improvements

* clean up NI_LineNames in tests

* remove DigitalWaveformFailure.data_index

* test___signal_with_line_names___change_line_names_property___signal_returns_new_line_name and _DigitalWaveformExtendedProperties

* signal_column_index

* line lengths

* italics for 'See "Signal index vs. signal column index"' notes

* add bitorder to from_port, and default to the industry standard 'big'

* rename to column_index

* bitorder != sys.byteorder

* add on_key_changed to ExtendedPropertyDictionary

* change tests to big-endian

* make on_key_changed private

* cleanup

* from typing_extensions import TypeAlias, to fix python 3.9

* fix pickling and copying issues with callbacks

* change on_key_changed to a list of weak methods, so ExtendedPropertyDictionaries can be shared by waveforms

---------

Co-authored-by: Mike Prosser <[email protected]>

* Fix unpickling issues with Waveform and Signal (#237)

* add versioned unpickling tests for waveform and signal, and fix issues causing the new tests to fail

* skip unpickle tests in oldest_deps test run

---------

Co-authored-by: Mike Prosser <[email protected]>

---------

Co-authored-by: Mike Prosser <[email protected]>
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.

ExtendedPropertyDictionary._on_key_changed breaks pickle compatibility

3 participants