Skip to content
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

breakdown of components in vpc.R #5

Open
dpastoor opened this issue Oct 23, 2014 · 3 comments
Open

breakdown of components in vpc.R #5

dpastoor opened this issue Oct 23, 2014 · 3 comments

Comments

@dpastoor
Copy link
Collaborator

After reading the code some it looks like we can 'break apart' these monolithic vpc functions and take a more modular approach to make maintaining and updating easier.

As it stands it looks like inside of vpc.R it handles:

  • data pre-processing
    • data from nonmem or other
    • get/set names for dv/idv/etc
    • data subsetting
    • set stratification?
  • data processing
    • binning (provided by auto_bin function)
    • summary statistic calculation(s)
  • plotting
    • base plot
    • stratifications
    • plot customizations (title, themes, etc)

The first thing I propose is we break apart the preproccessing, processing, 'base' plotting, and further customizations into individual functions, so the vpc() function would be little more than a wrapper along the lines of

vpc <- function(args...) {
preprocess_data() # set names, subset observations only, etc
process_data()   # data cleaning
summarize_data() # binning and summary stats
plot_vpc()  # 'base' plot 
customize_vpc() # additional customizations such as title, themes, etc
}

given the overlap across vpc functions this would also help reduce copied code etc...

Thoughts?

@ronkeizer
Copy link
Owner

you have a point. These vpc functions were clean at the start, but they have grown fast :) The general structure you list above is correct, and it is a good idea to extract the parts in single sub-function wherever possible. I think for the plot and "plot" and "customize" parts this should be relatively straightforward. Some parts of the pre_processing (e.g. checking/parsing arguments etc) can also be extracted as a general function, in fact this is already partially done in the function format_vpc_input_data().

However, the processing of data is more tricky, as it is fairly different between vpc for continuous, censored, categorical, and tte data. I propose to leave these functions alone for now and possibly come back to those later. We can start with the "plotting" and "customize" parts, as well as the "pre_processing". That would be a lot cleaner already.

@dpastoor
Copy link
Collaborator Author

Sounds good - I'll start extracting those components - do you want me to make a branch or just stick directly in the master. My thought is for these 'reorganization' parts just doing in master should be fine, but when we attempt a conversion to OO probably will branch in case sh*t hits the fan trying to figure things out.

@ronkeizer
Copy link
Owner

Master is fine, changes won't be dramatic and the package is in development stage anyhow.

This was referenced Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants