Added functionality to remove duplicate events #178
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR provides a basic fix for issue #169. Both
Sequence
andEventLibrary
now have aremove_duplicates
function that removes events that would be identical after rounding. The function is called both beforeSequence.write
and afterSequence.read
to ensure that no actual duplicate events exist in the event libraries.I opted not to do any rounding while adding events, because this affects all sequence analysis functions (e.g.
calculate_kspace
), and it affects any use ofget_block
to copy events (i.e. there can be compounding rounding errors if events are stored rounded). The proposed solution is also a lot faster.This does mean that in the example of #169, the library will still contain a lot of events, which will only be filtered when writing the sequence. Alternatively,
seq.remove_duplicates(in_place=True)
could be called manually at any point during the creation of a sequence to force rounding of the event libraries.In addition, I added a basis for
Sequence
unittests, testing whether sequences can be write and read back in, and recreated byadd_block(get_block(...))
, and still produce (within rounding errors) the same sequence (tested withget_block
,get_gradients
, andcalculate_kspace
). It also checks the .seq output compared to a known baseline, contained in thetests/expected_output
directory. The expected .seq output files can be regenerated with the current repository by setting a SAVE_EXPECTED environment variable before running pytest (e.g. if a change in the output happens that is confirmed to be correct).These tests should be expanded with further sequences snippets which tests all pypulseq functionality, and e.g. including all current example sequences.
Adding these tests uncovered two small bugs fixes in
Sequence
that I included here.