Fixture Overhaul

[jfear]

@daler I have played with different options trying to get preseq tests to pass with no luck. The tests work with another BAM that I had laying around. I think the best solution is to generate a larger FASTQ file.

This is a good opportunity to move from downloading to generating files on the fly using the fixture interface. We could then have a "big_bam" fixture that would write a larger fastq file, etc.

If you agree this is the right approach there are two big questions.

1. Where should fixtures live. I played with this sum and fixtures can be imported as any other module. So theoretically we could simply build another module (or add to lcdblib) a fixtures.py and import that into test from module.fixtures import *

2. How should we generate files. We could either use wrappers or hard code things themselves. Both methods have advantages and disadvantages.

   • Wrappers are nice because they are already set up (i.e. code and conda envs), but to use fixtures in other modules we would have to make this repository pip installable; also testing wrappers using test files generated by wrappers seems a little strange.
   • Hardcoded would be more portable, but then we would have to deal with environments and write all of the commands outs which seems silly.

[jfear]

If we choose to use the wrapper method; would it make sense to just use the output of the fixture directly in the test for that module. For example, if we have a sample1_se_bam fixture made using the hisat2 wrapper. For example:

Instead of:

def test_hisat2_align_se(hisat2_indexes, sample1_se_fq, tmpdir):
    d = _dict_of_hisat2_indexes(hisat2_indexes, '2L')
    indexes = list(d.values())
    snakefile = '''
        rule hisat2_align:
            input:
                fastq='sample1_R1.fastq.gz',
                index={indexes}
            output:
                bam='sample1.bam'
            log: "hisat2.log"
            wrapper: "file://wrapper"
    '''.format(indexes=indexes)
    d[sample1_se_fq] = 'sample1_R1.fastq.gz'
    input_data_func = symlink_in_tempdir(d)

    def check():
        assert "overall alignment rate" in open('hisat2.log').read()

        # should have at least some mapped and unmapped
        assert int(list(shell('samtools view -c -f 0x04 sample1.bam', iterable=True))[0]) > 0
        assert int(list(shell('samtools view -c -F 0x04 sample1.bam', iterable=True))[0]) > 0

    run(dpath('../wrappers/hisat2/align'), snakefile, check, input_data_func, tmpdir)

Just use the fixture output directly in the tests.

def test_hisat2_align_se(sample1_se_bam):
    assert int(list(shell('samtools view -c -f 0x04 {sample1_se_bam}', iterable=True))[0]) > 0
    assert int(list(shell('samtools view -c -F 0x04 {sample1_se_bam}', iterable=True))[0]) > 0

[daler]

I like this solution. There's a clean separation between the generating-of-content (always in a fixture; this is where the wrapper is called) and the testing-of-correctness (never in a fixture, only in test function).

Re: generating bams, flux simulator takes a fair amount of RAM/CPU/time just to generate those small fastqs we have, and it seems to be non-deterministic even when giving it a seed. So I think we should still download at least raw fastqs, the small genomic sequence (80kb of 2L), and annotations for the corresponding piece of the genome. But anything downstream of fastqs/fasta/annotation can be a fixture.

Re: preseq needing a bigger bam, might as well just host that file in the test data repo as long as you can get it down small enough.

Re: where fixtures should live, do you mean anything can be imported into conftest.py (which becomes globally avaliable), or each test_*.py file imports its own conftest.py replacement? For development convenience I'd rather have the fixtures in the same repo, but once things settle down we can pull them out into a separate module.

[jfear]

Sounds good, the more I have thought about the more I like this idea too.

I think it makes sense to have all fixtures to return a simple class object; The reason for the class is that there are sometimes more things we would want than just the path to the file. For testing we often want the path to a log. And for things like indices we really want the prefix. The cleanest way to do this is to munge everything in the fixture and then return a small class (a dict with dot access) which has the different paths loaded.

@pytest.fixture
def hisat2_align_se(hisat2_indexes, sample1_se_fq, tmpdir):
    d = _dict_of_hisat2_indexes(hisat2_indexes, '2L')
    indexes = list(d.values())
    snakefile = '''
        rule hisat2_align:
            input:
                fastq='sample1_R1.fastq.gz',
                index={indexes}
            output:
                bam='sample1.bam'
            log: "hisat2.log"
            wrapper: "file://wrapper"
    '''.format(indexes=indexes)
    d[sample1_se_fq] = 'sample1_R1.fastq.gz'
    input_data_func = symlink_in_tempdir(d)

    # need to make the run class return a tuple or dict or WrapperClass with paths
    bamPath , logPath = run(dpath('../wrappers/hisat2/align'), snakefile, input_data_func, tmpdir)

    return WrapperClass(bam=bamPath, log=logPath)

I agree with your above comments, did not realize that flux was resource hungry. I think it is good to store some datasets and pull them in.

As far as conftest.py, all I am saying is that it does not matter where you store fixtures. The magic of conftest is that pytest automatically imports a conftest.py file if it is in the directory. If we put the fixtures somewhere else we just need to import * them. If we made the lcdb-wrapper-tests a pip installable package, then we could use these fixtures in other packages from lcdb-wrapper-tests.test.conftest import * or something similar which I see as very valuable.

[daler]

Ah, I guess I thought there was more magic happening with conftest.py. I don't think I tried import * though, and couldn't figure out how to make it work with dotted names.

As far as class design, is there any additional behavior we might want besides what a dict can provide? I could see the class checking to make sure the files exist. More specifically, it would ensure that the paths provided don't have typos, since snakemake will raise an error if one of the output files were not created. But then we'd need a mechanism for disabling the os.path.exists check for non-path things like index prefixes. I can certainly be convinced otherwise, but I'm not sure we need the extra complexity of a class.

[jfear]

I think a dictionary would be sufficient, just thought accessing using dot notation would be more convenient.

[daler]

Thinking some more about this . . .

The readme files for wrappers are redundant with the test fixtures, which means that it's really easy for the readme to get out-of-sync with the tests and the wrapper itself. For example, some wrappers' readmes have examples for PE data but we don't have any PE tests yet.

I was initially thinking it would be amazing to have the fixtures be doctests of the readmes. But that would require too much overhead in the readme. Instead, I think it will be possible to have readmes generated from fixtures.

I was playing around with inspect and code objects to extract the snakefile string constant from fixtures, but this seems too hacky. I think we instead should move to a class-based fixture setup and have snakefile strings as attributes.

Here's a high-level overview of what I'm currently thinking:

• class to represent a wrapper example and fixture
• to reduce boilerplate, the class can handle most of the run(dpath(...)...) stuff. In the end a fixture needs:
  • input fixtures (one of which is tmpdir_factory)
  • snakefile
  • mapping of input fixtures to filenames expected by the snakefile
  • output
• one instance per example, so one instance per fixture. Might need a container-like class to represent the wrapper to group several examples that share setup?
• instance has snakefile attribute, which will be copied to docs
• instance has wrapper attribute, which is how it is "registered" to the correct wrapper and the wrapper docs can figure out which examples to use
• not sure how to make the @pytest.fixture work yet on classes/methods. Maybe just have a single method per instance that becomes the fixture?

It'll take some playing around with and will probably be a couple of iterations, but it could be nice.

[jfear]

haha, I am just glad I am not the only one making things difficult ;-)

I really like the idea of generating the READMEs from the fixtures, but not sure how to execute it. I have actually been reading the sphinx docs lately because I am wanting to be better about documenting some of my projects. Let me know if you need more input. I will think about it in the back of my head.

[daler]

Hah, I started down this rabbit hole because dupradar fails on sample2 but not sample1 example data when running rnaseq.snakefile. In order to have better tests for that, I parameterized the fastq-downloading fixture to run on both sample1 and sample2 with the intent of having that parameterization propagated out to all dependent fixtures. Then I realized the existing bam fixture is downloading from the test-data, breaking the dependency chain, so that needed re-writing. I started to rewrite it, then realized lots of other things are also downloading and also needere-writing. So I figured we should do some reorganization into separate files as we discussed here. And if we were doing an overhaul, might as well tackle the doctesting thing I've been thinking about for a while.

I'm imagining the examples would be an autogenerated markdown file that is separate from but linked to from the README. It would be independent of sphinx and would be created by a custom script to trawl the tests for fixture classes and build markdown files from the string in their snakemake attribute.

The hard part is figuring out how to make pytest work with classes and methods that are split into multiple files, how to wire up all the dependencies, the cleanest way to specify mapping of fixture to symlinked input file, and the cleanest way to specify output files. http://pytest.org/dev/fixture.html#using-fixtures-from-classes-modules-or-projects seems like the best place in the pytest docs. It'll be a bunch of trial-and-error, so I may try working on a toy example to figure out the details.