search

brainglobe / cellfinder-core

Standalone cellfinder cell detection algorithm
https://brainglobe.info/documentation/cellfinder/index.html
BSD 3-Clause "New" or "Revised" License
19 stars 16 forks source link

Decide whether to keep `connect_eight()` structure linking #111

Closed dstansby closed 1 year ago

dstansby commented 1 year ago

In the structure detection part of the cell detection code, there are two implementations of connections to merge structures together. Currently the default and only used one is connect_four(), which assumes two pixels are connected if they are directly next to each other (ie. in a plane, one pixel 'touches' four other ones).

There is also a connect_eight() bit of code, which is not tested, and I don't think a user can use: https://github.com/brainglobe/cellfinder-core/blob/f1188dfc8563a78c98713b260313c3a396a4bf62/src/cellfinder_core/detect/filters/volume/structure_detection.pyx#L177

This assumes two pixels are connected if they are next to each other, or their corners are touching (ie. in a plane, one pixel 'touches' eight others).

It would be good to decide if connect_eight() is worth keeping around - if so, we should add tests for it, and think about exposing it as an option for users. If not, I'd advocate for removing it to reduce the maintenance burden.

alessandrofelder commented 1 year ago

I would also advocate for its removal - fits in with the current efforts to reduce the maintenance burden - we can always dig it back up if we find a use case for it in the future. Any objections @adamltyson ?

adamltyson commented 1 year ago

If it's not used, it can go. It's not like we can't bring it back down the line if needed.