Closed sfmig closed 1 year ago
Thanks @sfmig!
Minor point: I wonder whether instead of putting questions for reviewers into the code itself and risking that we inadvertently merge them, should they in future be added as comments on the PR?
Yes please add all comments to the PR itself rather than in the code. And create issues rather than TODO:
etc. It's a bit of a pain for the developer, but much easier for everyone else to keep track.
On mypy
I've done some thinking and discussing with @willGraham01 (thanks!). We understood that the import errors are caused by the benchmarks code being outside the cellfinder_core
folder, so mypy
treats cellfinder_core
like a third-party library.
I would suggest to ignore any import errors happening in benchmarks/
by excluding them in pyproject.toml
(until we fix brainglobe/cellfinder#278 which is not high-priority).
A suite of asv benchmarks for timing the main imports of the package and several functions from the tools module. Benchmarks are defined as methods of a class, with common setup and teardown functions.
Basic usage
pip install asv
asv run
asv run --bench tools.prep.PrepTF
asv publish
and thenasv preview
.For further details on usage and useful commands, see the benchmarks/README.md file.
Structure
The structure follows the approach from the astropy-benchmarks repo, in which the benchmarks modules roughly mimic the package modules. The numpy benchmarks follow a slightly different structure but it is also a useful reference.
mypy fixes
To avoid mypy errors:
cellfinder_core.tools.prep.*
to the list of modules from which ignore import errors. Not sure if this is best practice but I couldn't find a better way around it. Happy to get feedback on it!Existing benchmarks
I moved a set of previous memory benchmarks written by @dstansby to a subdir in benchmarks called
mem_benchmarks
.