-
Notifications
You must be signed in to change notification settings - Fork 4
Rename functions/classes to be more R-standard #122
Copy link
Copy link
Open
Description
Proposal: align ModelArray function, class, and argument names with Advanced R style
Summary
This issue proposes a package-wide naming cleanup to make ModelArray more consistent with the naming guidance in Advanced R style:
- use
snake_casefor functions and arguments - reserve
PascalCasefor S4 classes - reserve
.for true S3 methods
The main intent is to make the API more idiomatic for R users without changing behavior.
Goals
- make exported function names more standard for R
- make argument names more readable and consistent
- reduce mixed conventions such as
camelCase,snake_case, and dotted names in the same API - preserve the
ModelArrayS4 class name, which is already appropriate
Compatibility proposal
Recommended transition strategy:
- Do a 1.0 release with 0.1.5 API, but with new features added
- Do a 26.0.0 release with the new API
Proposed public rename map
Classes and methods
| Current | Proposed | Notes |
|---|---|---|
ModelArray class |
ModelArray |
Keep as-is; S4 class name already follows PascalCase |
show("ModelArray") |
show("ModelArray") |
No change |
sources() |
sources() |
No change |
scalars() |
scalars() |
No change |
results() |
results() |
No change |
Exported functions
| Current | Proposed | Notes |
|---|---|---|
ModelArray() |
model_array() |
Constructor function becomes snake_case |
ModelArray.lm() |
fit_lm() |
Verb-based analysis entry point |
ModelArray.gam() |
fit_gam() |
Verb-based analysis entry point |
ModelArray.wrap() |
map_elements() |
Describes applying a user function element-wise |
analyseOneElement.lm() |
fit_lm_element() |
Likely deprecate then internalize |
analyseOneElement.gam() |
fit_gam_element() |
Likely deprecate then internalize |
analyseOneElement.wrap() |
map_element() |
Likely deprecate then internalize |
analysisNames() |
analysis_names() |
Snake_case accessor |
elementMetadata() |
element_metadata() |
Snake_case accessor |
exampleElementData() |
example_element_data() |
Snake_case helper |
gen_gamFormula_contIx() |
make_gam_formula_continuous_interaction() |
Expand abbreviation and use verb |
gen_gamFormula_fxSmooth() |
make_gam_formula_factor_smooth() |
Expand abbreviation and use verb |
h5summary() |
h5_summary() |
Snake_case |
mergeModelArrays() |
merge_model_arrays() |
Snake_case |
nElements() |
n_elements() |
Snake_case |
nInputFiles() |
n_input_files() |
Snake_case |
numElementsTotal() |
n_elements() |
Redundant public API; deprecate to alias |
scalarNames() |
scalar_names() |
Snake_case |
writeResults() |
write_results() |
Snake_case |
Proposed internal rename map
These are not necessarily user-facing, but cleaning them up would improve consistency across the package.
| Current | Proposed |
|---|---|
ModelArraySeed() |
model_array_seed() |
flagObjectExistInh5() |
h5_object_exists() |
flagResultsGroupExistInh5() |
h5_results_group_exists() |
flagAnalysisExistInh5() |
h5_analysis_exists() |
printAdditionalArgu() |
print_additional_arg() |
check_validity_correctPValue() |
check_p_adjust_methods() |
checker_gam_s() |
check_gam_s() |
checker_gam_t() |
check_gam_t() |
checker_gam_formula() |
check_gam_formula() |
bind_cols_check_emptyTibble() |
bind_cols_check_empty_tibble() |
print.h5summary() |
print.h5_summary() |
Proposed argument renames
Constructor and accessors
| Current | Proposed |
|---|---|
filepath |
path |
scalar_types |
scalar_names |
analysis_names |
analysis_names |
Analysis entry points
| Current | Proposed |
|---|---|
element.subset |
element_subset |
full.outputs |
full_output |
var.terms |
term_stats |
var.model |
model_stats |
var.smoothTerms |
smooth_term_stats |
var.parametricTerms |
parametric_term_stats |
correct.p.value.terms |
term_p_adjust |
correct.p.value.model |
model_p_adjust |
correct.p.value.smoothTerms |
smooth_term_p_adjust |
correct.p.value.parametricTerms |
parametric_term_p_adjust |
num.subj.lthr.abs |
min_subjects_abs |
num.subj.lthr.rel |
min_subjects_prop |
changed.rsq.term.index |
changed_rsq_term_index |
pbar |
progress |
FUN |
fn |
Per-element and writer helpers
| Current | Proposed |
|---|---|
i_element |
element_index |
num.stat.output |
n_stat_output |
flag_initiate |
init_only |
fn.output |
output_file |
df.output |
results_df |
modelarrays |
arrays |
phenotypes_list |
phenotypes_list or phenotypes_by_array |
merge_on |
by |
Questions for review
- Should we keep the analysis entry points as
fit_lm()/fit_gam(), or would namespaced generics likefit()be preferable long term? - Should
analyseOneElement.*remain exported at all, or should they become internal after a deprecation period? - Is
map_elements()the right replacement forModelArray.wrap(), or is there a better verb? - Should
scalar_typesbecomescalar_names, or is there a better term for that parameter? - Are there any existing downstream users who would need a longer deprecation window?
Suggested implementation scope
If there is support for this direction, implementation should include:
- exported functions
- internal helper functions
- argument names
- roxygen docs and
.Rdfiles - tests
- vignettes
- README examples
- pkgdown reference pages
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels