Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

Decide whether to keep connect_eight() structure linking #111

Closed
dstansby opened this issue Mar 29, 2023 · 2 comments · Fixed by #122
Closed

Decide whether to keep connect_eight() structure linking #111

dstansby opened this issue Mar 29, 2023 · 2 comments · Fixed by #122
Assignees
Labels
enhancement New feature or request

Comments

@dstansby
Copy link
Member

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:

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.

@dstansby dstansby added the enhancement New feature or request label Mar 29, 2023
@alessandrofelder
Copy link
Member

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
Copy link
Member

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

@dstansby dstansby self-assigned this Mar 30, 2023
@dstansby dstansby modified the milestone: 0.4.0 Mar 30, 2023
@dstansby dstansby moved this from Todo to In Progress in Core development Mar 31, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Core development Mar 31, 2023
willGraham01 pushed a commit that referenced this issue Aug 24, 2023
Remove old plugin engine v1 file
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants