Removal of Redundant Features in Libraries #minor #322
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #322 +/- ##
==========================================
+ Coverage 53.59% 58.93% +5.34%
==========================================
Files 30 29 -1
Lines 8426 7625 -801
Branches 3677 3135 -542
==========================================
- Hits 4516 4494 -22
+ Misses 1264 575 -689
+ Partials 2646 2556 -90
☔ View full report in Codecov by Sentry. |
|
Shouldn't we wait until we have E features merged from LibV7 before removing them? |
tests/test_basic.py
Outdated
| @@ -1435,8 +1435,8 @@ def test_spikecount_stimint1(): | |||
| spikecount_stimint + 2) | |||
|
|
|||
|
|
|||
| def test_spikecount_libv4peakindices(): | |||
| """basic: Test Spikecount in combination with LibV4 peak_indices""" | |||
| def test_spikecount_libv5peakindices(): | |||
There was a problem hiding this comment.
Shouldn't we just remove this test altogether with the DependencyV5_LibV4peakindices.txt file?
It is testing features that are no longer present.
There was a problem hiding this comment.
Yes you're right. Initially I thought this test is different and it adds more value however it is using the same data and retrieving the same result as in test_peak_indices.
There was a problem hiding this comment.
Ok I deleted that and added another test for setDependencyFileLocation since we need a test for checking the case when setDependencyFileLocation works.
Good point. Since there were also no tests for the E features I removed them in the PR but we can wait actually. Shall I revert that single commit? |
|
Yes, let's revert that commit, and wait for LibV7 features to remove them |
|
Alright merging... 🚀 |
anilbey
changed the title
Removal of Redundant Features in Libraries
Context:
This PR removes unused or untested features from active maintenance, aiming to cut down on the number of features we have to maintain and to make sure the ones we do keep are consistent in dealing with missing values, exceptions, and other edge cases that are currently compromising our scientific results.
It addresses the ongoing work documented in Issue #321.
Modifications:
LibV4has a single function calledpeak_indices.LibV5already has an improved version of it.depolarized_basein LibV3:LibV1::peak_indicesis removed, as theLibV5implementation is in use.LibV2::E6-E40protocols are neither utilised nor tested; the membrane systems team uses theLibV7implementation.LibV1::spike_width1is discarded, asLibV5::spike_half_widthis in use.LibV1::rest_voltage_valueis removed, asLibV5::voltage_baseis in use.LibV1::min_AHP_valuesis removed asLibV5::min_AHP_valuesis in use.LibV1::AHP_depth_absis removed, asLibV5::AHP_depth_absis in use.LibV1::min_AHP_indicesis removed, asLibV5::min_AHP_indicesis in use.Note:
While these features and functions are removed from active maintenance, it's important to note that their implementations remain accessible within the version control system, preserving the ability to revisit or reactivate them in future developments.