[r] Change default insertion mode for macs peak calling, rename function

bnprks / BPCells

Scaling Single Cell Analysis to Millions of Cells

https://bnprks.github.io/BPCells

Other

166 stars 17 forks source link

[r] Change default insertion mode for macs peak calling, rename function #143

Closed immanuelazn closed 3 weeks ago

immanuelazn commented 1 month ago

Deprecating call_macs_peaks() to call_peaks_macs() as discussed previously. Note, is this how we should be handling deprecated APIs?

immanuelazn commented 4 weeks ago

Cool, I'll make the changes with the better way of styling!

I think it would make sense to do the multithreading as a separate PR as that requires work more on the internal logic, rather than interface/documentation

immanuelazn commented 3 weeks ago

Styling changes from deprecation finished!

bnprks commented 3 weeks ago

Overall, this looks quite good and is ready to merge.

Something we should discuss separately: do we knew why we missed the default argument issue initially despite comparing outputs against ArchR. Do you think our checks didn't cover this or weren't running with insertion_mode set to the default?

immanuelazn commented 3 weeks ago

Yup, so the results were compared in this file . I passed in the argument for both in insertion_mode within this test case but didn't change the default param on the macs, and bedfile themselves.