Repository: popsim-consortium/stdpopsim (language: python)

Issue:
can't load recomb maps from DroMel/ComeronCrossoverV2_dm6
**Short description of the problem:**

The tarball is successfully downloaded, but the individual files don't get extracted.

**How to reproduce the problem:**

```python
>>> import logging
>>> import stdpopsim
>>>
>>> logging.basicConfig(level=logging.DEBUG)
>>>
>>> species = stdpopsim.get_species("DroMel")
>>> genetic_map = species.get_genetic_map("ComeronCrossoverV2_dm6")
>>>
>>> first_chrom = species.genome.chromosomes[0]
>>> genetic_map.get_chromosome_map(first_chrom.id)
INFO:stdpopsim.cache:Downloading https://stdpopsim.s3-us-west-2.amazonaws.com/genetic_maps/DroMel/comeron2012v2_maps.tar.gz
DEBUG:stdpopsim.cache:Checking SHA256
DEBUG:stdpopsim.cache:Extracting /home/grg/.cache/stdpopsim/tmpvnjdzbjg/downloaded
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/grg/src/stdpopsim/stdpopsim/genetic_maps.py", line 97, in get_chromosome_map
    self.download()
  File "/home/grg/src/stdpopsim/stdpopsim/genetic_maps.py", line 83, in download
    self._cache.download()
  File "/home/grg/src/stdpopsim/stdpopsim/cache.py", line 152, in download
    utils.untar(local_path, extract_dir)
  File "/home/grg/src/stdpopsim/stdpopsim/utils.py", line 113, in untar
    raise ValueError(f"Tarball format error: member {info.name} not a file")
ValueError: Tarball format error: member comeron2012v2_maps not a file
```


**Suggested fixes:**

I think the problem is that the tarball has a folder inside it, and we currently only allow flat file structure. The fact that this wasn't caught earlier is clearly a failure of our tests.


Discussion / hints:

Fix patch:
diff --git a/stdpopsim/catalog/DroMel/genetic_maps.py b/stdpopsim/catalog/DroMel/genetic_maps.py
index de515d9dc..59af6a56c 100644
--- a/stdpopsim/catalog/DroMel/genetic_maps.py
+++ b/stdpopsim/catalog/DroMel/genetic_maps.py
@@ -55,7 +55,7 @@
         "DroMel/comeron2012v2_maps.tar.gz"
     ),
     sha256="79484e6042c36b06946af672cabf2ec3012b6bfc6e0018517c8e7be4769cd334",
-    file_pattern="genetic_map_comeron2012v2_dm6_chr{id}.txt",
+    file_pattern="comeron2012v2_maps/genetic_map_comeron2012v2_dm6_chr{id}.txt",
     citations=[
         stdpopsim.Citation(
             author="Comeron et al",
diff --git a/stdpopsim/genetic_maps.py b/stdpopsim/genetic_maps.py
index 3825d2466..2bb40f367 100644
--- a/stdpopsim/genetic_maps.py
+++ b/stdpopsim/genetic_maps.py
@@ -101,7 +101,13 @@ def get_chromosome_map(self, id):
         # needs to be redownloaded.
         map_file = self.map_cache_dir / self.file_pattern.format(id=chrom.id)
         if map_file.exists():
-            recomb_map = msprime.RateMap.read_hapmap(map_file)
+            recomb_map = msprime.RateMap.read_hapmap(
+                map_file,
+                rate_col=2,
+                # TODO: set the sequence length. Unfortunately, some of our
+                # maps are shorter than our chromosomes, so this will fail.
+                # sequence_length=chrom.length
+            )
         else:
             warnings.warn(
                 "Recombination map not found for chromosome: '{}'"
diff --git a/stdpopsim/utils.py b/stdpopsim/utils.py
index b60f55e37..f7b5d487f 100644
--- a/stdpopsim/utils.py
+++ b/stdpopsim/utils.py
@@ -109,7 +109,7 @@ def untar(filename, path):
             # Due to security concerns, we only extract tarballs containing a
             # very restrictive set of file types. See the warning here:
             # https://docs.python.org/3/library/tarfile.html#tarfile.TarFile.extractall
-            if not info.isfile():
+            if not (info.isfile() or info.isdir()):
                 raise ValueError(f"Tarball format error: member {info.name} not a file")
             if info.name.startswith("/") or info.name.startswith(".."):
                 raise ValueError(f"Refusing to extract {info.name} outside of {path}")


Test patch:
diff --git a/tests/test_genetic_maps.py b/tests/test_genetic_maps.py
index 87e5cb808..82ede7a18 100644
--- a/tests/test_genetic_maps.py
+++ b/tests/test_genetic_maps.py
@@ -7,6 +7,8 @@
 import tempfile
 import os.path
 import pathlib
+import pytest
+import re
 
 import msprime
 
@@ -235,3 +237,33 @@ def test_unknown_chromosome(self):
         for bad_chrom in ["", "ABD", None]:
             with self.assertRaises(ValueError):
                 genetic_map.get_chromosome_map(bad_chrom)
+
+    @pytest.mark.filterwarnings(
+        "ignore: Recombination map.*is longer than chromosome length"
+    )
+    def test_one_chrom_from_each_map(self):
+        for gm in stdpopsim.all_genetic_maps():
+            species = gm.species
+            if gm.id in ("NaterPA_PonAbe2", "NaterPP_PonAbe2"):
+                # XXX: these maps are currently broken:
+                #   ValueError: The last entry in the 'rate' column must be zero
+                continue
+            # Just load the first chromosome in the list.
+            # There's no requirement that any given chromosome is actually
+            # in the map, and we don't have a direct way to check
+            # for its presence. But if this chromsome is *not* in the
+            # map, we will recieve a warning (and then fail the test below).
+            chrom = species.genome.chromosomes[0]
+            with pytest.warns(None) as record:
+                gm.get_chromosome_map(chrom.id)
+
+            # Fail the test if we get any warnings matching the message below.
+            # There doesn't seem to be a way to simply check for the absense
+            # of a specific warning using pytest, so we record all warnings
+            # and check manually.
+            record = [
+                r
+                for r in record
+                if re.match(r"Recombination map not found", str(r.message)) is not None
+            ]
+            assert len(record) == 0, f"{species.id} / {gm.id}"