-
Notifications
You must be signed in to change notification settings - Fork 22
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
Add array type handling for normalization #835
Add array type handling for normalization #835
Conversation
Very happy about your input on any of the questions I raised in the comments @flying-sheep :) |
Also interested what @nicolassidoux thinks here! :) |
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.
- You added lots of
NotImplementedError
messages. I wonder whether we can DRY this more easily? - Those error messages should also always suggest which types are support for that function as a sort of solution.
adata_to_norm_casted = adata_to_norm.copy()
why do we always need those copies? Doesn't the fixture already create new copies?
Yess
Good point, the dynamic registries help here again
because I just not paying attention to this yet, I'll remove :) |
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
I addressed the feedback raised in the discussions. Hope I didn't miss any. I'll merge this now but if I forgot something @eroell please make me aware. Thank you! |
Description of changes
Implements different array types for normalization functions which serve as example and guidance on how we implement this for all other functions.
Technical details
Uses singledispatch. Inspired from e.g. scanpy's scale; but less performance engineering, rather focusing on leveraging frameworks such as dask-ml.