Partial matching

[MichaelChirico]

Follow-up to #252

• All tests now pass under strict requirements for partial matching
• The .Rout.save files were updated to reflect the current state of the package so that R CMD check passes

[therneau]

This is nice work. I was genuinely surprised at the number of fixes in the package code, many oversights and a few wrong memories: I actually thought that the model.frame argument was weight not weights, and seq_along was also a surprise.

The aareg function's use of coefficient rather than coefficients is an oversight that I never noticed before; but that function really should not be used as the timreg package is much better. I've long thought of deprecating it.

In the end the only one I object to is expansion of iter to iter.max in the test suite. (I should have named that arg "iter" in the first place!) If CRAN later decides to be formal the change will bite only me, not any users.

[MichaelChirico]

Since some of those iter= changes made it in already in #252, WDYT about merging this PR without that change, and I'll file a follow-up that reverts all iter.max= usage back to iter= in the test suite?

[MichaelChirico]

seq(along=) might be an older practice from before seq_along() was added as a function?

{survival} is far from the only package using that style, here's 14K hits for the same on GitHub:

https://github.com/search?q=lang%3AR%20%2Fseq%5B(%5Dalong%5Cs*%3D%2F&type=code

[MichaelChirico]

PS there wound up being a lot of .Rout.save files that don't have any substantive update (since I just copied over every .Rout file from R CMD check), would you like me to try & revert those to keep the diff here smaller?

[therneau]

I'm still grappling with how best to use git, so have patience while I work that out, in particular how to pull things down to my test bed and run checks there before I accept something. One important thing I forgot to check with yours is that the true source for any .R file that starts with a comment "Auto-generated from noweb" is the .Rnw files in the noweb directory; changing one of those .R files is useless since it wil be recreated the next time I do (cd noweb; make fun). This is over half your .R changes.
I reverted the iter.max changes within a local branch, but have not pushed it up the chain yet. I need to think on your and my arguments some more to be sure it's not just a "stiff neck" on my part. (I used a sed script, so it's fairly easy to go back and forth.)

[remlapmot]

To conveniently pull down and test a pull request before accepting it - I find it easiest to use a graphical Git client like GitHub Desktop which avoids having to remember git command line syntax.

https://docs.github.com/en/desktop/working-with-your-remote-repository-on-github-or-github-enterprise/viewing-a-pull-request-in-github-desktop#opening-a-pull-request-branch-in-github-desktop

[remlapmot]

GitHub Desktop is freely available https://desktop.github.com/.

[MichaelChirico]

I'm still grappling with how best to use git, so have patience while I work that out, in particular how to pull things down to my test bed and run checks there before I accept something.

@remlapmot suggestion to use a GUI is good. For the command line, I guess you might want something like:

# Register my fork as a remote locally, I choose name 'MC'
git remote add MC git@github.com:MichaelChirico/survival.git
# Pull down the branch used to make this PR
git fetch MC partial-matching
# checkout the code in this branch to be able to run tests with my code changes applied
git checkout partial-matching
# <make changes if needed>
# <git commit as elsewhere>
# you have rights to push code to my branch:
git push MC partial-matching

[therneau]

I have just pushed an update that incorporates all but iter.max, and passes R CMD check.