ergm icon indicating copy to clipboard operation
ergm copied to clipboard

Extract common Roxygen documentation patterns into a submodule.

Open krivit opened this issue 3 years ago • 11 comments

Several packages, in particular ergm, ergm.ego, tergm, and ergm.multi have common arguments, such as formula, response, basis, etc.. To avoid code duplication between packages, it would be helpful to create a common set of Roxygen templates that can then be shared via a Git submodule.

These, in turn, should, in theory, be possible to keep separate from package-specific templates by putting them into a subdirectory of man-roxygen/, e.g., man-roxygen/ergm/ for those inherited from the ergm package.

The question is whether this is actually possible. I've experimentally placed one template into a subdirectory in branch template_subdir_test: https://github.com/statnet/ergm/blob/ea5ed5177840313a72ad46dd4a447788b3b90ab9/R/param_names.R#L30-L34

Running devtools::document() on this branch works on Linux. @mbojan , @chad-klumb , @martinamorris , @sgoodreau , can I please trouble (at least) one of you to check that it works on Windows, which uses \ as the directory separator?

krivit avatar Apr 08 '21 07:04 krivit

It works on my Windows machine. I deleted man/ergm_model.Rd and running devtools::document() generates it properly with canonical argument documented from the template.

Adding a subfolder to man-roxygen as a submodule should work. Of course one needs to remember to initialize/update these when cloning the repo. Perhaps there should be a note in README about that. I think CI scripts should not require any update because they do not Roxygenize anything.

mbojan avatar Apr 09 '21 13:04 mbojan

With the merger of the new documentation API, it would now be even more helpful to be able to share out man-roxygen/ from ergm, since it now contains parameter documentation and other templates. I see two approaches:

Note: In the discussion below, I need to distinguish directory paths from GitHub repositories, so the repository names are prefixed with "github:".

Dedicated repository

This is the "correct" way to do this: create a repository, e.g., github:statnet/ergm-man-roxygen and import it into subdirectory, say, man-roxygen/ergm/ of all of the packages for which it might be of interest. Then, @template ergm/formula.R would be used by all packages to reference the formula template. ergm package itself may also have a man-roxygen/ergm/ subdirectory, or it could rely entirely on the repository and mount the submodule on man-roxygen/.

This is the cleanest and most economical approach.

Mount github:statnet/ergm

A more crude approach is to import the whole ergm source as a submodule of man-roxygen, and the template could then be referenced as, e.g., @template ergm/man-roxygen/formula.R. This has the advantage of requiring no additional repositories or modification to the ergm source tree. It has the downside of very cumbersome submodule names (because git submodule doesn't let you check out just a subdirectory), and every repository using this submodule having to check out something close to a copy of the ergm package repository, increasing the size of the working copy by about 30MB according to my experiments.

I'm testing this approach out in github:statnet/ergm.rank@submodule-test.

It may be possible to get away with slightly less cumbersome @templates by using symbolic links. That is, we could mount github:statnet/ergm onto, e.g., submodule/ergm and then symlink man-roxygen/ergm to point at ../submodule/ergm/man-roxygen, so that @template ergm/formula.R. This should work on all modern OSes, and Git does track symbolic links. I haven't tested this yet.

I am testing this approach out in github:statnet/ergm.rank@submodule-test.

Can Windows/MacOS folks confirm that these work? One unfortunate consequence is that you need to run

git submodule init
git submodule update

after checking out the branch and

git submodule update

to pull in the latest github:statnet/ergm.

krivit avatar Sep 04 '21 12:09 krivit

Further to this, the symbolic link approach does not appear to work unless the submodule is initialised and checked out, because R CMD build will attempt to dereference links and will stop on a broken link---which happens if submodule is not initialised. (Ideally, this should not prevent the package from building, just Roxygen from running.)

krivit avatar Sep 05 '21 06:09 krivit

Reported to R team: https://bugs.r-project.org/show_bug.cgi?id=18186.

krivit avatar Sep 05 '21 06:09 krivit

R team responded that it's a known issue, but there isn't currently enough time to fix it. I've investigated what's involved, and while it's doable, it's also modifying a very critical part of R that has to work on a wide variety of platforms. Also, even if accepted, it will be years until we can actually use the fix in practice.

The only practical solution is to instruct the developer to create the symlink after they initialise the submodule.

I am increasingly leaning towards the "correct" approach of having a dedicated submodule repository, particularly since the plan is to modularise ergm in the first place. I'll test it out on a branch.

krivit avatar Nov 04 '21 11:11 krivit

I've created a template repository statnet/ergm-man-roxygen by making a copy of statnet/ergm, deleting all branches but master, all commits not affecting the man-roxygen/ directory, and moving the man-roxygen/ directory to repository root.

For those who want to test the new setup, here it is:

  • statnet/ergm@man-roxygen-submodule has a version of ergm that mounts statnet/ergm-man-roxygen on man-roxygen/.
  • statnet/ergm.rank@submodule-test has a version of ergm.rank mounts statnet/ergm-man-roxygen on man-roxygen/ergm (to allow it to have its own templates if needed).

@mbojan , @chad-klumb , @drh20drh20 , @joycecheng , can you please test this out?

krivit avatar Nov 12 '21 02:11 krivit

@mbojan , @chad-klumb , @drh20drh20 , could I please get some feedback from a Windows/RStudio user? If the workflow is unmanageable, I need to know ASAP.

krivit avatar Dec 03 '21 01:12 krivit

i can try to test on windows as well, but i'm not sure what to do to test things.

martinamorris avatar Dec 03 '21 01:12 martinamorris

i can try to test on windows as well, but i'm not sure what to do to test things.

That's the thing: I don't know what the Windows+RStudio workflow is for submodules. That's why I pinged the coders. :-)

krivit avatar Dec 03 '21 02:12 krivit

RStudio (all platforms) does not know anything about git submodules. Using the "Git" tab and its buttons will only pull changes in the main repo. If the submodule state changes it's content may not be updated. One needs to drop to shell (Tools>Shell) or open RStudio Terminal and initialize/update the submodule from the commandline:

  • git submodule init upon the first checkout, then
  • git submodule update to fetch the contents of the submodule.

@martinamorris , you will have to follow the steps above.

@krivit , one thing I didn't think of earlier. One can switch to the man-roxygen-submodule branch, initialize the submodule, fetch it and so on, but to switch back to master (or any other branch) one has to manually delete man-roxygen. git will not do that for you, as with normal folders. Perhaps an alternative of subtree-merging will be more attractive than submodules (apologies that it did not occur to me earlier)? The clear advantage is that it will not mess-up the repo and will not require any extra steps upon checkouts or pulls (affecting users, CI etc.). The potential disadvantage is that the ergm repo (and any other "child repos" containing the "common" man-roxygen) will contain it's full history instead of just a reference to a particular commit. As with submodules each child repo has to be updated as soon as the man-roxygen is updated. IMHO worth considering.

Apart from that:

Package installed correctly although I'm seeing the following log:

Install log
* installing to library 'C:/Users/mbojanowski/Documents/R/library/x86_64-w64-mingw32/4.1-dev'
* installing *source* package 'ergm' ...
** using staged installation
** libs
make: Nothing to be done for 'all'.
installing to C:/Users/mbojanowski/Documents/R/library/x86_64-w64-mingw32/4.1-dev/00LOCK-ergm/00new/ergm/libs/x64
** R
** data
** inst
** byte-compile and prepare package for lazy loading
** help
*** installing help indices
  converting help for package 'ergm'
    finding HTML links ... done
    B-ergmTerm                              html  
    BDStratTNT-ergmProposal                 html  
Rd warning: no hsearch.rds meta data for package ergm
    Bernoulli-ergmReference                 html  
    CondB1Degree-ergmProposal               html  
Rd warning: no hsearch.rds meta data for package ergm
    CondB2Degree-ergmProposal               html  
Rd warning: no hsearch.rds meta data for package ergm
    CondDegree-ergmProposal                 html  
Rd warning: no hsearch.rds meta data for package ergm
    CondDegreeDist-ergmProposal             html  
Rd warning: no hsearch.rds meta data for package ergm
    CondDegreeMix-ergmProposal              html  
Rd warning: no hsearch.rds meta data for package ergm
    CondInDegree-ergmProposal               html  
Rd warning: no hsearch.rds meta data for package ergm
    CondInDegreeDist-ergmProposal           html  
Rd warning: no hsearch.rds meta data for package ergm
    CondOutDegree-ergmProposal              html  
Rd warning: no hsearch.rds meta data for package ergm
    CondOutDegreeDist-ergmProposal          html  
Rd warning: no hsearch.rds meta data for package ergm
    ConstantEdges-ergmProposal              html  
Rd warning: no hsearch.rds meta data for package ergm
    Curve-ergmTerm                          html  
    DiscUnif-ergmProposal                   html  
Rd warning: no hsearch.rds meta data for package ergm
    DiscUnif-ergmReference                  html  
    DiscUnif2-ergmProposal                  html  
Rd warning: no hsearch.rds meta data for package ergm
    DiscUnifNonObserved-ergmProposal        html  
Rd warning: no hsearch.rds meta data for package ergm
    DistRLE-ergmProposal                    html  
Rd warning: no hsearch.rds meta data for package ergm
    Dyads-ergmConstraint                    html  
    Exp-ergmTerm                            html  
    F-ergmTerm                              html  
    HammingConstantEdges-ergmProposal       html  
Rd warning: no hsearch.rds meta data for package ergm
    HammingTNT-ergmProposal                 html  
Rd warning: no hsearch.rds meta data for package ergm
    Label-ergmTerm                          html  
    Log-ergmTerm                            html  
    NodematchFilter-ergmTerm                html  
    Offset-ergmTerm                         html  
    Prod-ergmTerm                           html  
    S-ergmTerm                              html  
    StdNormal-ergmProposal                  html  
Rd warning: no hsearch.rds meta data for package ergm
    StdNormal-ergmReference                 html  
    Sum-operator-ergmTerm                   html  
    Symmetrize-ergmTerm                     html  
    TNT-ergmProposal                        html  
Rd warning: no hsearch.rds meta data for package ergm
    Unif-ergmProposal                       html  
Rd warning: no hsearch.rds meta data for package ergm
    Unif-ergmReference                      html  
    UnifNonObserved-ergmProposal            html  
Rd warning: no hsearch.rds meta data for package ergm
    absdiff-ergmTerm                        html  
    absdiffcat-ergmTerm                     html  
    altkstar-ergmTerm                       html  
    anova.ergm                              html  
    approx.hotelling.diff.test              html  
    finding level-2 HTML links ...
    as.network.numeric                      html   done

    as.rlebdm.ergm                          html  
    as.rlebdm.ergm_conlist                  html  
    asymmetric-ergmTerm                     html  
    atleast-ergmTerm                        html  
    atmost-ergmTerm                         html  
    attrcov-ergmTerm                        html  
    b1concurrent-ergmTerm                   html  
    b1cov-ergmTerm                          html  
    b1degrange-ergmTerm                     html  
    b1degree-ergmTerm                       html  
    b1degrees-ergmConstraint                html  
    b1dsp-ergmTerm                          html  
    b1factor-ergmTerm                       html  
    b1mindegree-ergmTerm                    html  
    b1nodematch-ergmTerm                    html  
    b1sociality-ergmTerm                    html  
    b1star-ergmTerm                         html  
    b1starmix-ergmTerm                      html  
    b1twostar-ergmTerm                      html  
    b2concurrent-ergmTerm                   html  
    b2cov-ergmTerm                          html  
    b2degrange-ergmTerm                     html  
    b2degree-ergmTerm                       html  
    b2degrees-ergmConstraint                html  
    b2dsp-ergmTerm                          html  
    b2factor-ergmTerm                       html  
    b2mindegree-ergmTerm                    html  
    b2nodematch-ergmTerm                    html  
    b2sociality-ergmTerm                    html  
    b2star-ergmTerm                         html  
    b2starmix-ergmTerm                      html  
    b2twostar-ergmTerm                      html  
    balance-ergmTerm                        html  
    bd-ergmConstraint                       html  
    blockdiag-ergmConstraint                html  
    blocks-ergmConstraint                   html  
    call.ErgmTerm                           html  
    check.ErgmTerm                          html  
    cohab                                   html  
    coincidence-ergmTerm                    html  
    concurrent-ergmTerm                     html  
    concurrentties-ergmTerm                 html  
    control.ergm                            html  
    control.ergm.bridge                     html  
    control.ergm.godfather                  html  
    control.gof                             html  
    control.san                             html  
    control.simulate.ergm                   html  
    ctriple-ergmTerm                        html  
    cycle-ergmTerm                          html  
    cyclicalties-ergmTerm                   html  
    cyclicalweights-ergmTerm                html  
    ddsp-ergmTerm                           html  
    degcor-ergmTerm                         html  
    degcrossprod-ergmTerm                   html  
    degrange-ergmTerm                       html  
    degree-ergmTerm                         html  
    degree1.5-ergmTerm                      html  
    degreedist-ergmConstraint               html  
    degreedist                              html  
    degrees-ergmConstraint                  html  
    density-ergmTerm                        html  
    desp-ergmTerm                           html  
    dgwdsp-ergmTerm                         html  
    dgwesp-ergmTerm                         html  
    dgwnsp-ergmTerm                         html  
    diff-ergmTerm                           html  
    dnsp-ergmTerm                           html  
    dot-dyads-ergmConstraint                html  
    dsp-ergmTerm                            html  
    dyadcov-ergmTerm                        html  
    dyadnoise-ergmConstraint                html  
    dyadnoise-ergmProposal                  html  
Rd warning: no hsearch.rds meta data for package ergm
    dyadnoiseTNT-ergmProposal               html  
Rd warning: no hsearch.rds meta data for package ergm
    ecoli                                   html  
    edgecov-ergmTerm                        html  
    edges-ergmConstraint                    html  
    edges-ergmTerm                          html  
    egocentric-ergmConstraint               html  
    enformulate.curved-deprecated           html  
    equalto-ergmTerm                        html  
    ergm-defunct                            html  
    ergm-deprecated                         html  
    ergm-errors                             html  
    ergm-internal                           html  
    ergm-options                            html  
    ergm-package                            html  
    ergm-parallel                           html  
    ergm                                    html  
    ergm.allstats                           html  
    ergm.bridge.llr                         html  
    ergm.design                             html  
    ergm.estfun                             html  
    ergm.eta                                html  
    ergm.exact                              html  
    ergm.geodistdist                        html  
    ergm.getnetwork                         html  
    ergm.godfather                          html  
    ergm.mple                               html  
    ergmConstraint                          html  
Rd warning: no hsearch.rds meta data for package ergm
    ergmHint                                html  
Rd warning: no hsearch.rds meta data for package ergm
    ergmKeyword                             html  
    ergmMPLE                                html  
    ergmProposal                            html  
Rd warning: no hsearch.rds meta data for package ergm
    ergmReference                           html  
Rd warning: no hsearch.rds meta data for package ergm
    ergmTerm                                html  
Rd warning: no hsearch.rds meta data for package ergm
    ergm_Cstate_clear                       html  
    ergm_MCMC_sample                        html  
    ergm_SAN_slave                          html  
    ergm_bd_init                            html  
    ergm_dyadgen_select                     html  
    ergm_keyword                            html  
    ergm_mk_std_op_namewrap                 html  
    ergm_model                              html  
    ergm_plot.mcmc.list                     html  
    ergm_preprocess_response                html  
    ergm_propagate_ext.encode               html  
    ergm_proposal                           html  
    ergm_proposal_table                     html  
    ergm_state                              html  
    ergm_symmetrize                         html  
    ergmlhs                                 html  
    esp-ergmTerm                            html  
    eut-upgrade                             html  
    faux.desert.high                        html  
    faux.dixon.high                         html  
    faux.magnolia.high                      html  
    faux.mesa.high                          html  
    fix.curved                              html  
    fixallbut-ergmConstraint                html  
    fixedas-ergmConstraint                  html  
    florentine                              html  
    g4                                      html  
    get.node.attr                           html  
    geweke.diag.mv                          html  
    gof                                     html  
    greaterthan-ergmTerm                    html  
    gwb1degree-ergmTerm                     html  
    gwb1dsp-ergmTerm                        html  
    gwb2degree-ergmTerm                     html  
    gwb2dsp-ergmTerm                        html  
    gwdegree-ergmTerm                       html  
    gwdsp-ergmTerm                          html  
    gwesp-ergmTerm                          html  
    gwidegree-ergmTerm                      html  
    gwnsp-ergmTerm                          html  
    gwodegree-ergmTerm                      html  
    hamming-ergmConstraint                  html  
    hamming-ergmTerm                        html  
    idegrange-ergmTerm                      html  
    idegree-ergmTerm                        html  
    idegree1.5-ergmTerm                     html  
    idegreedist-ergmConstraint              html  
    idegrees-ergmConstraint                 html  
    ininterval-ergmTerm                     html  
    intransitive-ergmTerm                   html  
    is.curved                               html  
    is.dyad.independent                     html  
    is.inCH                                 html  
    is.valued                               html  
    isolatededges-ergmTerm                  html  
    isolates-ergmTerm                       html  
    istar-ergmTerm                          html  
    kapferer                                html  
    kstar-ergmTerm                          html  
    localtriangle-ergmTerm                  html  
    logLik.ergm                             html  
    logLikNull                              html  
    m2star-ergmTerm                         html  
    mcmc.diagnostics                        html  
    meandeg-ergmTerm                        html  
    mm-ergmTerm                             html  
    molecule                                html  
    mutual-ergmTerm                         html  
    nearsimmelian-ergmTerm                  html  
    network.list                            html  
    nodal_attributes-API                    html  
    nodal_attributes                        html  
    nodecov-ergmTerm                        html  
    nodecovar-ergmTerm                      html  
    nodefactor-ergmTerm                     html  
    nodeicov-ergmTerm                       html  
    nodeicovar-ergmTerm                     html  
    nodeifactor-ergmTerm                    html  
    nodematch-ergmTerm                      html  
    nodemix-ergmTerm                        html  
    nodeocov-ergmTerm                       html  
    nodeocovar-ergmTerm                     html  
    nodeofactor-ergmTerm                    html  
    nparam                                  html  
    nsp-ergmTerm                            html  
    nvattr.copy.network                     html  
    observed-ergmConstraint                 html  
    odegrange-ergmTerm                      html  
    odegree-ergmTerm                        html  
    odegree1.5-ergmTerm                     html  
    odegreedist-ergmConstraint              html  
    odegrees-ergmConstraint                 html  
    opentriad-ergmTerm                      html  
    ostar-ergmTerm                          html  
    param_names                             html  
    predict.formula                         html  
    randomtoggle-ergmProposal               html  
Rd warning: no hsearch.rds meta data for package ergm
    rank_test.ergm                          html  
    receiver-ergmTerm                       html  
    rlebdm                                  html  
    samplk                                  html  
    sampson                                 html  
    san                                     html  
    search.ergmConstraints                  html  
    search.ergmProposals                    html  
    search.ergmReferences                   html  
    search.ergmTerms                        html  
    sender-ergmTerm                         html  
    simmelian-ergmTerm                      html  
    simmelianties-ergmTerm                  html  
    simulate.ergm                           html  
    simulate.formula                        html  
    smalldiff-ergmTerm                      html  
    smallerthan-ergmTerm                    html  
    snctrl                                  html  
    sociality-ergmTerm                      html  
    sparse-ergmHint                         html  
    spectrum0.mvar                          html  
    strat-ergmHint                          html  
    sum-ergmTerm                            html  
    summary.ergm                            html  
REDIRECT:topic	 Previous alias or file overwritten by alias: C:/Users/mbojanowski/Documents/R/library/x86_64-w64-mingw32/4.1-dev/00LOCK-ergm/00new/ergm/help/InitWtErgmTerm.sum.html
    summary.ergm_model                      html  
    summary.formula                         html  
    summary_formula                         html  
    threetrail-ergmTerm                     html  
    to_ergm_Cdouble                         html  
    transitive-ergmTerm                     html  
    transitiveties-ergmTerm                 html  
    transitiveweights-ergmTerm              html  
    triadcensus-ergmTerm                    html  
    triangle-ergmTerm                       html  
    tripercent-ergmTerm                     html  
    ttriple-ergmTerm                        html  
    twopath-ergmTerm                        html  
    update.network                          html  
    wrap.ergm_model                         html  
    wtd.median                              html  
** building package indices
** installing vignettes
** testing if installed package can be loaded from temporary location
** testing if installed package can be loaded from final location
** testing if installed package keeps a record of temporary installation path
* DONE (ergm)

in which:

  • Rd warning: no hsearch.rds meta data for package ergm -- I'm guessing the static HTML pages for the dynamic parts may be broken. I don't remember why R on Windows builds them anyway...
  • REDIRECT:topic (...) -- I did not investigate, but should be fixable

In RStudio though the man pages for terms, proposals and constraints seem to be working just fine.

I did not manage to test the indexes of pages from other packages yet (ergm.rank), but will do.

mbojan avatar Dec 03 '21 08:12 mbojan

Yet another approach can be as with workshop common content (see statnet/admin): the repo ergm-man-roxygen has a GH action that, upon change, commits the updates to man-roxygen folders of other repos. The advantage of this approach is that we can have package-specific content of man-roxygen (appearing only in, say, ergm, but not ergm.rank etc.).

mbojan avatar Dec 03 '21 08:12 mbojan