-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Finish updating docstrings #232
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #232 +/- ##
==========================================
+ Coverage 92.19% 92.24% +0.04%
==========================================
Files 37 37
Lines 6623 6639 +16
Branches 1590 1590
==========================================
+ Hits 6106 6124 +18
Misses 298 298
+ Partials 219 217 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, there are just a couple of small things:
- I think the description of the spatial tree is a bit off right now
- multiprocessing description needs a little ironing in one place, it mentioned
combine
as somewhat optional which might give the wrong idea (i think you still need to pass a method, even if that method is an identity map)
I think I left some other comments but those will be mostly just passing thoughts
@KasiaKoz bump to avoid this going stale! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Updated docstrings I missed in #229 and added type hints to methods if they have an associated docstring. Now everything will render nicely in the docs :)