Delete unused code & add tests #18
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #18 +/- ##
===========================================
+ Coverage 64.04% 79.68% +15.63%
===========================================
Files 33 19 -14
Lines 840 576 -264
===========================================
- Hits 538 459 -79
+ Misses 302 117 -185 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I can't speak for all of the tests as I'm not entirely sure what they're all meant to be doing (maybe we need an issue for adding docstrings to this package, maybe a docs build that can be handed off to the website?).
But otherwise, more tests = better. One remark on a comment that's been removed but doesn't seem to have been addressed?
| @@ -29,11 +29,9 @@ def get_cells( | |||
| cells_only: bool = False, | |||
| cell_type: Optional[int] = None, | |||
| ): | |||
| # TODO: implement csv read | |||
There was a problem hiding this comment.
Did we... actually address this? The function isn't changed
There was a problem hiding this comment.
No, but there's no need to.
There was a problem hiding this comment.
@willGraham01 beat me to the punch, but everything looks good to me!
|
@willGraham01 I've raised #19 Adding docstrings is something the entirety of BG sorely needs. |
|
Thanks both. Going to release a new version, so yell at me if anything breaks! |
Description
What is this PR
Why is this PR needed?
There is a lot of code in this repo that is no longer used by any BrainGlobe tool. Keeping it in the repo increases the maintenance burden.
What does this PR do?
Removes unused code and adds some more tests for what remains.
Also closes #12
How has this PR been tested?
Please explain how any new code has been tested, and how you have ensured that no existing functionality has changed.
Is this a breaking change?
Hopefully not. I've checked other BG repos for usage of deleted code, and nothing came up. This process isn't foolproof though, so we may just have to release and hope for the best! This is a fairly dangerous approach, so happy to hear of alternatives.
Does this PR require an update to the documentation?
If any features have changed, or have been added. Please explain how the documentation has been updated (and link to the associated PR). See here for details.
Checklist: