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

FT: Fix and consolidate code in maars-lm and ols-summary #59

Open
7 of 10 tasks
ricfog opened this issue Mar 28, 2021 · 0 comments
Open
7 of 10 tasks

FT: Fix and consolidate code in maars-lm and ols-summary #59

ricfog opened this issue Mar 28, 2021 · 0 comments

Comments

@ricfog
Copy link
Collaborator

ricfog commented Mar 28, 2021

General improvements:

  • we should always return by default all estimates that have been computed through comp_var. Cases like this should be avoided. We can use this code to check which estimates are available in comp_var. If everything is set to FALSE, then the all available elements are returned
  • there are multiple (3-5) functions that call the cli package and contain only a few lines of code. We should remove these functions and insert their code directly into the functions where they are actually used. However, before going ahead with this consolidation we should make sure that each of these functions is used only once

Clean/fix/more documentation:

  • potentially change the name of the functioncheck_fn_args_summary because it is very similar to [check_fn_args](https://github.com/shamindras/maars/blob/iss-55-post-demo-fixes/R/utils-common.R#L92). Also check whether this information is correct
  • add more examples in the documentation of get_summary
  • get_assumptions should return a tibble
  • comp_var needs more examples in its documentation
  • remove the lines of code relative to stats::lm in summary.maars_lm because the corresponding output is not printed
  • rename get_mms_summary_print_lm_style and do not print anything within the function. This function should only return the summary table with the significance codes

Consolidation:

  • remove get_mms_rnm_cols_suff. This function was created in an old iteration of the code and is not used anymore
  • remove fetch_mms_comp_var_attr. There is almost no synergy at play in this function, which currently serves as a very simple wrapper for many other functions. This only makes the dependencies between functions we need to be aware of more complicated. We should be calling those specific functions directly (up for a [short] discussion given @shamindras cares about this a lot :) )
ricfog added a commit that referenced this issue Mar 29, 2021
ricfog added a commit that referenced this issue Mar 29, 2021
ricfog added a commit that referenced this issue Mar 29, 2021
…int but only returns the summary table with stars
ricfog added a commit that referenced this issue Mar 29, 2021
ricfog added a commit that referenced this issue Mar 29, 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

1 participant