-
Notifications
You must be signed in to change notification settings - Fork 0
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
remove all plotting functions? #17
Comments
good thinking, definitely always recommend that, that's the main reason we moved all plotting from isoreader into isoprocessor to make a more streamlined package with one primary purpose rather than feature overload |
I've been stripping stuff out and doing some refactoring. What are your thoughts on wrapper functions? e.g. delta_values(), which is a wrapper for abundance_ratios, abundance_ratios, little_deltas, bulk_and_clumping_deltas. |
Hi @japhir . I don't like wrapper functions because they obscure functionality to the user and make it harder to understand what's going when someone reads a knitted RMarkdown report. Ideally in reports the commands all make it very clear what's going on with each function verbified if appropriate, e.g. As for plotting functions, if they are just wrappers I try to avoid them, if they provide some helpful functionality that's very frequently used for the subject data I think they're okay. I'm still on the fence for the plotting functions in isoprocessor which ones to keep and which ones to ultimately retire. just my two cents, hope that helps! |
I was considering that it may be smart to get rid of all the functions that I don't use a lot before this (hopefully) starts getting used by other people, so there will be less code to maintain and the package will be more focussed on functionality in stead of feature overload. What do you think, @sebkopf?
The text was updated successfully, but these errors were encountered: