-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
calcium refactoring #54
Conversation
c16dd98
to
47c3423
Compare
def robust_std(self, x): | ||
''' | ||
Robust estimate of std | ||
def _robust_std(self, x): |
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.
Why making this more protected? Can it be moved into a utility module, so that other parts of the code base can use it?
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.
my first motivation was to hide the class from the documentation. it's not intended AFAIK to be used elsewhere.
agreed it should probably get pulled out into a separate function to improve modularity & testing, even if not into a separate module yet.
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.
Approved. Keep an eye out for functions that might be useful in other places; it we start seeing many of them, I suggest refactoring to a utilities module.
thanks @nicain !! |
this PR addresses #48
todo: