feat: add check for unscaled ElectricalSeries data#650
Conversation
Add `check_electrical_series_unscaled_data` to warn when ElectricalSeries has integer data type with default conversion (1.0) and offset (0.0), which suggests raw acquisition units are not properly scaled to Volts. The check also considers `channel_conversion` for per-channel scaling to avoid false positives when proper conversion factors are specified. Includes documentation for the best practice and changelog entry. Closes #408
h-mayorquin
left a comment
h-mayorquin
left a comment
There was a problem hiding this comment.
Two minor points and this should be good to go.
Also, I am thinking that the schema might be improved to reduce the likelihood of errors:
NeurodataWithoutBorders/nwb-schema#671
| assert result is not None | ||
| assert "int16" in result.message | ||
|
|
||
| def test_pass_with_empty_data(self): |
There was a problem hiding this comment.
I feel that we should not allow ElectricalSeries with empty data and that this test might break or conflict with a check for non-empty data. What do you think?
| ) | ||
| result = check_electrical_series_unscaled_data(electrical_series) | ||
| assert result is not None | ||
| assert "int16" in result.message |
There was a problem hiding this comment.
The convention I have seen here in this repo is to test against the full message.
There was a problem hiding this comment.
Actually, I pushed this to avoid some work for you. Let me know if you want me to revert it.
for more information, see https://pre-commit.ci
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #650 +/- ##
==========================================
+ Coverage 77.64% 81.44% +3.79%
==========================================
Files 47 47
Lines 1673 1708 +35
==========================================
+ Hits 1299 1391 +92
+ Misses 374 317 -57
🚀 New features to boost your workflow:
|
3 participants
Add
check_electrical_series_unscaled_datato warn when ElectricalSeries has integer data type with default conversion (1.0) and offset (0.0), which suggests raw acquisition units are not properly scaled to Volts.The check also considers
channel_conversionfor per-channel scaling to avoid false positives when proper conversion factors are specified.Includes documentation for the best practice and changelog entry.
Closes #408
Closes #395