change the way pattern name list is generated

[jeanettejohnson]

This is addressing a bug report I received over slack.

I was able to reproduce the issue and saw it came from line 484, which is supposed to make a list of the name of the top pattern for each gene. It produced the error shown above.

I commented that line and re-wrote the list comprehension logic as a simple for loop that compiles the list of all pattern names, and confirmed that this produced the expected output with no errors.

My best guess as to the root cause: Possible change of base functionality in R version? I think maybe something that used to return a list now returns a list of lists, but I am not positive. The code I re-wrote generates a list, which the next line expects as input to compute the final statistic. I do not expect this change to affect any other part of the code.

[dimalvovs]

Hey, I was not able to reproduce the issue on master:

══ Testing test_patternMarkers.R ═════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
[ FAIL 0 | WARN 0 | SKIP 0 | PASS 0 ]
This is CoGAPS version 3.21.5 
Running Standard CoGAPS on GIST.data_frame (1363 genes and 9 samples)
[ FAIL 0 | WARN 0 | SKIP 0 | PASS 1 ] Done!

can it be data-specific? let's consider amending the test_patternMarkers.R until it fails on master, add a fix and show that is works in feature branch.

[dimalvovs]

I am still not able to reproduce this issue, @jeanettejohnson could you share code that use used to reproduce? I have tried the following two tests and both finish without error

test_that("patternMarkers work with threshold = 'all' for general mode", {
    #set up
    data(GIST)
    res <- CoGAPS(GIST.data_frame, nIterations=1000,
                  seed=1, messages=FALSE)

    test <- patternMarkers(res, threshold = "all")
    expect_gt(length(test$PatternMarkers), 0)
})

test_that("patternMarkers work with threshold = 'all' for sparse mode", {
    #set up
    data(GIST)
    res <- CoGAPS(GIST.data_frame, nIterations=1000, 
                  seed=1, messages=FALSE, sparseOptimization=TRUE)

    test <- patternMarkers(res, threshold = "all")
    expect_gt(length(test$PatternMarkers), 0)
})

[dimalvovs]

We could not create a test that fails, but the change passes regression tests. I removed a couple of things, please merge if it looks ok.