Skip to content

Conversation

@danielinteractive
Copy link
Collaborator

closes #3

Hi @friendly, thanks a lot for this nice package. I am proposing two small improvements in this PR:

  1. Automatically omit strata with a single observation in CMHtest() because they do not contribute to the test statistics and currently cause the computations to return no results.
  2. Use the generalized Moore-Penrose inverse from MASS in CMHtest() such that it can work when the variance
    matrix is singular.

I added also a bunch of tests for CMHtest. The test cases are courtesy of the https://github.com/PSIAIMS/CAMIS/ project, namely @statasaurus, @DrLynTaylor, @yannickvandendijck and @loganjohnson0. I have tried to appropriately call out their authorship in the test file.

Please let me know your comments, looking forward to your review :-)

Best, Daniel

@friendly
Copy link
Owner

friendly commented Nov 9, 2025

Thanks for this, Daniel
The changes to CMHtest() you propose seem useful.
I haven't used testthat very much before and I don't use air, so I'm hoping this doesn't interfere with my setup.
I'll review this over the next few days.

@danielinteractive
Copy link
Collaborator Author

Thanks a lot @friendly !

No worries, the air config file won't interfere with anything else if you don't use air yourself.
The testthat usage should not be a problem and will just be part of the usual R CMD check.

@danielinteractive
Copy link
Collaborator Author

Hi @friendly , kind reminder to have a quick look at this if you get the chance, thanks!

@friendly friendly merged commit 5b7f892 into friendly:master Nov 18, 2025
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

Successfully merging this pull request may close these issues.

CMHtest throws an uncatchable "Error in solve.default(AVA)"

2 participants