Skip to content

Add spliceinto! function.#338

Merged
jakobnissen merged 3 commits intoBioJulia:masterfrom
jakobnissen:insert
Jul 16, 2025
Merged

Add spliceinto! function.#338
jakobnissen merged 3 commits intoBioJulia:masterfrom
jakobnissen:insert

Conversation

@jakobnissen
Copy link
Copy Markdown
Member

@jakobnissen jakobnissen commented Jul 16, 2025

Fixes #336

@pdimens - I'm interested in feedback. Does this cover your use case, and what do you think of the API?
The reason for the deviation from splice! is:

  • It bugs me that 4:3 == 5:4, and yet passing these equal arrays into splice! causes different behaviour
  • It's unnecessary and inefficient that splice! returns the removed sequence

By the way, if you need this function now and can't wait for the 3.5.0 release (which will happen once this PR is merged), or need compat with older versions of BioSequences, you can use the implmentation from this PR. It's efficient, and uses no internals.

This function inserts a sequence into a biosequence, and optionally deletes part
of the original sequence.
The naming difference from `Base.splice!` reflects is slightly different API.
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.75%. Comparing base (95d9218) to head (cde0fa8).
Report is 22 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #338      +/-   ##
==========================================
+ Coverage   90.87%   91.75%   +0.88%     
==========================================
  Files          31       29       -2     
  Lines        2400     2827     +427     
==========================================
+ Hits         2181     2594     +413     
- Misses        219      233      +14     
Flag Coverage Δ
unittests 91.75% <100.00%> (+0.88%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jakobnissen
Copy link
Copy Markdown
Member Author

Another option: Just have it be Base.insert! as you suggested, anyway. The API would then be insert!(seq, 3:5, x), where x must have the same length as 3:5, else it will throw an error.
Or, we could use insert!, but allow their lengths to differ.

@pdimens
Copy link
Copy Markdown

pdimens commented Jul 16, 2025

Thanks, I'll check this in <3hrs. I think having the insertion be a range is redundant, no? Can you see a use case of inserting a collection not at startposition:start+len?

@pdimens
Copy link
Copy Markdown

pdimens commented Jul 16, 2025

Nvm, I read the commit (currently on my phone) and see the optional deletion of destination

@pdimens
Copy link
Copy Markdown

pdimens commented Jul 16, 2025

Would it be fair to alias it as indel!, since that's what you've effectively done?

@jakobnissen
Copy link
Copy Markdown
Member Author

Yeah, that's also a cool name! Maybe even better.

@pdimens
Copy link
Copy Markdown

pdimens commented Jul 16, 2025

The spliceinto!(BioSequences, range, other) works perfectly

@jakobnissen jakobnissen merged commit ba3a960 into BioJulia:master Jul 16, 2025
22 checks passed
@pdimens
Copy link
Copy Markdown

pdimens commented Jul 16, 2025

Thank you!

@jakobnissen jakobnissen deleted the insert branch July 16, 2025 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

method for insert!() to add multiple nucleotides

2 participants