Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unit testing to work with msprime > 0.7.4. #522

Closed
wants to merge 1 commit into from

Conversation

grahamgower
Copy link
Member

Closes #518.

@codecov
Copy link

codecov bot commented May 9, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@c19fd69). Click here to learn what that means.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #522   +/-   ##
=========================================
  Coverage          ?   99.63%           
=========================================
  Files             ?       21           
  Lines             ?     1901           
  Branches          ?      202           
=========================================
  Hits              ?     1894           
  Misses            ?        2           
  Partials          ?        5           
Impacted Files Coverage Δ
stdpopsim/models.py 100.00% <100.00%> (ø)
stdpopsim/cli.py 98.01% <0.00%> (ø)
stdpopsim/catalog/canis_familiaris.py 100.00% <0.00%> (ø)
stdpopsim/slim_engine.py 99.54% <0.00%> (ø)
stdpopsim/species.py 100.00% <0.00%> (ø)
stdpopsim/catalog/e_coli.py 100.00% <0.00%> (ø)
stdpopsim/qc/pongo_abelii_qc.py 100.00% <0.00%> (ø)
stdpopsim/engines.py 100.00% <0.00%> (ø)
stdpopsim/genetic_maps.py 100.00% <0.00%> (ø)
stdpopsim/qc/homo_sapiens_qc.py 100.00% <0.00%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c19fd69...6ecd5ff. Read the comment docs.

@grahamgower
Copy link
Member Author

There's something wrong with the codecov report. This should definitely have failed, as I added a code path without a test for it.

@grahamgower
Copy link
Member Author

I added a test for the version compatibility shim, and checked it under both msprime 0.7.4, and git master.

$ python -c "import msprime; print(msprime.__version__)"
0.7.4
$ rm .coverage
$ nosetests --with-coverage --cover-package stdpopsim tests
...............................................................................................................................................................................................................................................................................................................................................S
Name                                           Stmts   Miss  Cover
------------------------------------------------------------------
stdpopsim/__init__.py                             21      2    90%
stdpopsim/_version.py                              1      0   100%
stdpopsim/cache.py                                16      0   100%
stdpopsim/catalog/__init__.py                      0      0   100%
stdpopsim/catalog/arabidopsis_thaliana.py         41      0   100%
stdpopsim/catalog/canis_familiaris.py             15      0   100%
stdpopsim/catalog/drosophila_melanogaster.py      52      0   100%
stdpopsim/catalog/e_coli.py                        8      0   100%
stdpopsim/catalog/homo_sapiens.py                332      0   100%
stdpopsim/catalog/pongo_abelii.py                 40      0   100%
stdpopsim/citations.py                            48      0   100%
stdpopsim/cli.py                                 302      2    99%
stdpopsim/engines.py                              56      0   100%
stdpopsim/genetic_maps.py                         83      1    99%
stdpopsim/genomes.py                              55      0   100%
stdpopsim/models.py                              190      0   100%
stdpopsim/qc/__init__.py                           4      0   100%
stdpopsim/qc/arabidopsis_thaliana_qc.py           42      0   100%
stdpopsim/qc/drosophlia_melanogaster_qc.py        36      0   100%
stdpopsim/qc/homo_sapiens_qc.py                  258      0   100%
stdpopsim/qc/pongo_abelii_qc.py                   25      0   100%
stdpopsim/slim_engine.py                         223      0   100%
stdpopsim/species.py                              73      0   100%
stdpopsim/utils.py                                16      0   100%
------------------------------------------------------------------
TOTAL                                           1937      5    99%
----------------------------------------------------------------------
Ran 336 tests in 44.248s

OK (SKIP=1)

$ python -c "import msprime; print(msprime.__version__)"
0.7.5.dev215+ng2ccf262
$ rm .coverage
$ nosetests --with-coverage --cover-package stdpopsim tests
...............................................................................................................................................................................................................................................................................................................................................S
Name                                           Stmts   Miss  Cover
------------------------------------------------------------------
stdpopsim/__init__.py                             21      2    90%
stdpopsim/_version.py                              1      0   100%
stdpopsim/cache.py                                16      0   100%
stdpopsim/catalog/__init__.py                      0      0   100%
stdpopsim/catalog/arabidopsis_thaliana.py         41      0   100%
stdpopsim/catalog/canis_familiaris.py             15      0   100%
stdpopsim/catalog/drosophila_melanogaster.py      52      0   100%
stdpopsim/catalog/e_coli.py                        8      0   100%
stdpopsim/catalog/homo_sapiens.py                332      0   100%
stdpopsim/catalog/pongo_abelii.py                 40      0   100%
stdpopsim/citations.py                            48      0   100%
stdpopsim/cli.py                                 302      2    99%
stdpopsim/engines.py                              56      0   100%
stdpopsim/genetic_maps.py                         83      1    99%
stdpopsim/genomes.py                              55      0   100%
stdpopsim/models.py                              190      0   100%
stdpopsim/qc/__init__.py                           4      0   100%
stdpopsim/qc/arabidopsis_thaliana_qc.py           42      0   100%
stdpopsim/qc/drosophlia_melanogaster_qc.py        36      0   100%
stdpopsim/qc/homo_sapiens_qc.py                  258      0   100%
stdpopsim/qc/pongo_abelii_qc.py                   25      0   100%
stdpopsim/slim_engine.py                         223      0   100%
stdpopsim/species.py                              73      0   100%
stdpopsim/utils.py                                16      0   100%
------------------------------------------------------------------
TOTAL                                           1937      5    99%
----------------------------------------------------------------------
Ran 336 tests in 44.207s

OK (SKIP=1)

@grahamgower grahamgower requested a review from jeromekelleher May 9, 2020 19:03
@jeromekelleher
Copy link
Member

Nicely done @grahamgower, thanks. Another option would be to back out of the change in msprime where we dropped the unused argument. It's no harm, and we could mark it for removal some time in the future after msprime 1.0 and we've transitioned stdpopsim to using asdict() like you say.

Otherwise, we'll still have the problem that people with older versions of stdpopsim will break when msprime 1.0 is released.

@jeromekelleher
Copy link
Member

I don't mind breaking test code, but we can't break production code. So, backing out of msprime change seems simplest?

@grahamgower
Copy link
Member Author

Sure, that's fine too. Note also that the behaviour of msprime.SimpleBottleneck changed: it previously had a default for the population attribute, but this seems to have been removed.

@jeromekelleher
Copy link
Member

Reverted the change to msprime in tskit-dev/msprime#1038

See tskit-dev/msprime#1036 and tskit-dev/msprime#1037 for longer term plan.

@jeromekelleher
Copy link
Member

Sure, that's fine too. Note also that the behaviour of msprime.SimpleBottleneck changed: it previously had a default for the population attribute, but this seems to have been removed.

Is the SimpleBottleneck used here somewhere? (It was never documented, so I felt OK to change it). Can you open an issue on msprime to track the problem if so, please?

@grahamgower grahamgower deleted the get-ll-rep branch May 26, 2020 07:32
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.

Tests no longer pass with msprime update
2 participants