-
Notifications
You must be signed in to change notification settings - Fork 27
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
CORP decomposition wrapper #811
Comments
Just checking, I assume you mean wrapper (as in a function transformer or higher order function) and not decorator? Decorators may cause forced nesting, whereas a wrapper on its own is okay. If you're going the function wrapper route and CORP is fairly generic, I'd just have the one wrapper and cater to edge cases as needed. If you feel CORP is very metric dependent, then maybe have the one wrapper offload/route (e.g. using a dict -> score mapping or similar) to a more metric specific wrappers for each score. For an initial implementation this may be fine, but if it is potentially impacting a lot of the codebase then please consider some additional thoughts I had below: Alternative design: Its harder to track side-effects and structure that may be necessary when using wrappers (which store state internally) - classes however can dictate how much visibility an object should have based on its structure. I'm not sure of the exact details but for example if CORP requires defining MSB, DSC and UNC for a particular score, that can be abstracted in the class definition, initialization and/or using builder patterns e.g.
This allows you to:
|
Currently we can use the scores package to calculate the CORP decomposition https://www.pnas.org/doi/10.1073/pnas.2016191118#sec-4
It would be nice to have a wrapper to do this for us to remove the manual steps.
One design choice that needs to be made is, do we:
The text was updated successfully, but these errors were encountered: