-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add read/write support for wave reference waves in JSON wave notes #1989
Add read/write support for wave reference waves in JSON wave notes #1989
Conversation
Results from review from @MichaelHuth:
|
7ea6447
to
f3646d7
Compare
We would fail later on due to the IsNumericWave/IsTextWave checks anyway but the error message is better.
92ebc5a
to
e28ad6a
Compare
Ready for review. |
Looks good to me. The extra Make to do the null wave check seems to be not very efficient, though there doesn't seem to be an obvious better method. I thought of FindValue but that does not accept WREF waves. |
I've removed the make invocation and used our $ git range-diff @{u}...HEAD
1: e28ad6af0 ! 1: 695662449 MIES_JSONWaveNotes.ipf: Add read/write support for wave reference waves
@@ Packages/MIES/MIES_JSONWaveNotes.ipf: threadsafe Function/WAVE JWN_GetTextWaveFr
+
+ JSON_Release(jsonID)
+
-+ Make/FREE/N=(numRows) junk = WaveExists(container[p])
-+
-+ ASSERT_TS(Sum(junk) == numRows, "Encountered invalid waves in the container")
++ ASSERT_TS(IsNaN(GetRowIndex(container, refWave = $"")), "Encountered invalid waves in the container")
+
+ return container
+End |
e28ad6a
to
6956624
Compare
No description provided.