Issue #1755 #1758
Issue #1755 #1758
Conversation
|
@Matyasz Can we add a regression test for the case where |
Codecov Report
@@ Coverage Diff @@
## master #1758 +/- ##
=======================================
Coverage 36.16% 36.16%
=======================================
Files 346 346
Lines 33817 33819 +2
=======================================
+ Hits 12229 12232 +3
+ Misses 21588 21587 -1
Continue to review full report at Codecov.
|
|
Let's add a test case for this. Test that 1. warning was raised and 2. data properly truncated Especially since this came from accidentally leaving out the truncation in the first place :) edit: didn't see Nick's request. Yes please add a unit test but also use the pytest warning capture as well |
| def test_get_running_df_one_fewer_timestamp_check_warning(running_data, | ||
| timestamps, | ||
| lowpass): | ||
| with pytest.warns(UserWarning): |
There was a problem hiding this comment.
For this, I would do:
with pytest.warns(UserWarning) as w:
And then test that the captured warning msg string starts with: "Time array is 1 value shorter than encoder array."
|
|
||
| assert len(output) == 4 | ||
| # Check that the output is actually trimmed | ||
| assert len(output) == len(timestamps) - 1 |
There was a problem hiding this comment.
I would add the following asserts:
assert output["v_sig"] == running_data["items"]["behavior"]["encoders"]["vsig"][:-1]
assert output["v_in"] == running_data["items"]["behavior"]["encoders"]["vin"][:-1]
Which checks that encoder values are truncated AND have the expected values.
…r timestamp than encoder. Updated tests for this method to nsure that the correct warning is presented in this case, and that it returns a trimmed dataset.
Overview:
LIMS has been experiencing a number of failures over the past two days related to a small bug. Apparently, during a refactor, a particular warning got carried over, but not the action that it warned about. Doug has added full context to this in a comment on the GitHub issue.
Addresses:
Addresses issue #1755
Type of Fix:
functionality to not work as expected)
Solution:
Just trimming off the last entry of a list. Doug responded to the issue with some context, "Camstim has long had an issue where it generates one extra encoder reading at the end." Apparently this happens every time with mesoscope.
Changes:
Validation:
Screenshots:
Unit Tests:
Script to reproduce error and fix:
Configuration details:
Checklist
Allen Institute Contribution Guidelines
Numpy Standards
appropriate
rc/x.y.z as its merge target