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

Structure splitting: Smoke test and refactor for numba #167

Closed alessandrofelder closed 1 year ago

alessandrofelder commented 1 year ago

Description

This PR adds

Why is this PR needed?

Our tests don't currently cover the case where cell detection requires structure splitting.

What does this PR do?

See description.

References

Closes #166, closes #158, closes #159

How has this PR been tested?

Run tests locally and on CI.

Does this PR require an update to the documentation?

New test and fixtures have docstrings, test data documented in docstring of new testing module.

Checklist:

alessandrofelder commented 1 year ago

Note that CI fails in expected way, ensuring our tests now can reproduce #158.

adamltyson commented 1 year ago

@alessandrofelder would it be worth renaming the directory containing the new test data? The dataset label doesn't mean much to anyone other than me. Maybe something like structure_split_test, then just add a note (maybe a txt file) to highlight which dataset it's from for our records?

alessandrofelder commented 1 year ago

Thanks for your comments, both!

Re test data, I had that thought too, but decided against it as

But maybe it's worth reconsidering this decision? We definitely want to be using pooch soon!

dstansby commented 1 year ago

👍 sounds good re. test data

dstansby commented 1 year ago

Nice - hopefully code coverage should increase when this commit runs on the main branch!