Repository: vanheeringen-lab/genomepy

Issue:
Please provide genomes_dir error after upgrade to 0.8.1
In the config `genome_dir` seems to be renamed to `genomes_dir`. When I upgrade my genomepy version it does not add `genomes_dir` to the config, and gives me this error.
```
Traceback (most recent call last):
  File "/home/sande/anaconda3/envs/test-snakemake-wrappers/bin/genomepy", line 10, in <module>
    sys.exit(cli())
  File "/home/sande/anaconda3/envs/test-snakemake-wrappers/lib/python3.8/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/home/sande/anaconda3/envs/test-snakemake-wrappers/lib/python3.8/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/home/sande/anaconda3/envs/test-snakemake-wrappers/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/sande/anaconda3/envs/test-snakemake-wrappers/lib/python3.8/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/sande/anaconda3/envs/test-snakemake-wrappers/lib/python3.8/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/home/sande/anaconda3/envs/test-snakemake-wrappers/lib/python3.8/site-packages/genomepy/cli.py", line 195, in install
    genomepy.install_genome(
  File "/home/sande/anaconda3/envs/test-snakemake-wrappers/lib/python3.8/site-packages/genomepy/functions.py", line 256, in install_genome
    generate_env()
  File "/home/sande/anaconda3/envs/test-snakemake-wrappers/lib/python3.8/site-packages/genomepy/functions.py", line 154, in generate_env
    for env in generate_exports():
  File "/home/sande/anaconda3/envs/test-snakemake-wrappers/lib/python3.8/site-packages/genomepy/functions.py", line 120, in generate_exports
    for name in list_installed_genomes():
  File "/home/sande/anaconda3/envs/test-snakemake-wrappers/lib/python3.8/site-packages/genomepy/functions.py", line 104, in list_installed_genomes
    genomes_dir = get_genomes_dir(genomes_dir, check_exist=False)
  File "/home/sande/anaconda3/envs/test-snakemake-wrappers/lib/python3.8/site-packages/genomepy/utils.py", line 231, in get_genomes_dir
    raise exceptions.ConfigError("Please provide or configure a genomes_dir")
norns.exceptions.ConfigError: Please provide or configure a genomes_dir
```

Up to you to decide if you want to have `genomes_dir` existing in parallel to `genome_dir` for backward compatability, or you just overwrite it :)

The easy fix is for me is to add `genomes_dir` to `~/.config/genomepy/genomepy.yaml`.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 658ad3d..df6cb8b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 
+### Fixed
+- backwards compatibility with old configuration files (with `genome_dir` instead of `genomes_dir`)
+- updating the README.txt will only happen if you have write permission
+- after gzipping files the original unzipped file is now properly removed
+- providers will only download genome summaries when specifically queried
+- updated available blacklists (added GRCh38)
+
 ## [0.8.1] - 2020-05-11
 
 ### Added
@@ -25,6 +32,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
     - STAR and HISAT2 will now generate splice-aware indexes if annotation files are available.
 
 ### Changed
+- `Genome.props` has been renamed to `Genome.plugin`
 - sizes no longer a plugin, but always gets executed
 - `genomepy FUNCTION --help` texts expanded
 - all genomepy classes exported when imported into Python
diff --git a/genomepy/cli.py b/genomepy/cli.py
index 6563894..f599055 100755
--- a/genomepy/cli.py
+++ b/genomepy/cli.py
@@ -135,11 +135,18 @@ def get_install_options():
     add provider in front of the provider specific options to prevent overlap"""
     install_options = general_install_options
 
-    for name in genomepy.ProviderBase.list_providers():
-        p_dict = genomepy.ProviderBase.create(name).list_install_options()
-        for option in p_dict.keys():
-            p_dict[option]["long"] = name + "-" + p_dict[option]["long"]
-        install_options.update(p_dict)
+    # only run if -h or --help are in the arguments
+    if "-h" in click.get_os_args() or "--help" in click.get_os_args():
+        for name in genomepy.ProviderBase.list_providers():
+            # directly access the provider's arguments, skips __init__
+            p_dict = eval(
+                "genomepy.provider."
+                + name.capitalize()
+                + "Provider.list_install_options(None)"
+            )
+            for option in p_dict.keys():
+                p_dict[option]["long"] = name + "-" + p_dict[option]["long"]
+            install_options.update(p_dict)
 
     return install_options
 
diff --git a/genomepy/genome.py b/genomepy/genome.py
index 8e6bd66..2c96252 100644
--- a/genomepy/genome.py
+++ b/genomepy/genome.py
@@ -186,7 +186,7 @@ class Genome(Fasta):
             metadata.get("provider", "na") == "na"
             or "tax_id" not in metadata
             or "assembly_accession" not in metadata
-        ):
+        ) and os.access(self.readme_file, os.W_OK):
             self._update_metadata(metadata)
             write_readme(self.readme_file, metadata, lines)
 
diff --git a/genomepy/plugins/blacklist.py b/genomepy/plugins/blacklist.py
index 6af3308..6f16f33 100644
--- a/genomepy/plugins/blacklist.py
+++ b/genomepy/plugins/blacklist.py
@@ -9,15 +9,17 @@ from genomepy.plugin import Plugin
 
 
 class BlacklistPlugin(Plugin):
-    base_url = "http://mitra.stanford.edu/kundaje/akundaje/release/blacklists/"
+    stanford_url = "http://mitra.stanford.edu/kundaje/akundaje/release/blacklists/"
+    encode_url = "https://www.encodeproject.org/files/"
     http_dict = {
-        "ce10": base_url + "ce10-C.elegans/ce10-blacklist.bed.gz",
-        "dm3": base_url + "dm3-D.melanogaster/dm3-blacklist.bed.gz",
-        "hg38": base_url + "hg38-human/hg38.blacklist.bed.gz",
-        "hg19": base_url
+        "ce10": stanford_url + "ce10-C.elegans/ce10-blacklist.bed.gz",
+        "dm3": stanford_url + "dm3-D.melanogaster/dm3-blacklist.bed.gz",
+        "hg19": stanford_url
         + "hg19-human/wgEncodeHg19ConsensusSignalArtifactRegions.bed.gz",
-        "mm9": base_url + "mm9-mouse/mm9-blacklist.bed.gz",
-        "mm10": base_url + "mm10-mouse/mm10.blacklist.bed.gz",
+        "hg38": encode_url + "ENCFF356LFX/@@download/ENCFF356LFX.bed.gz",
+        "GRCh38": encode_url + "ENCFF356LFX/@@download/ENCFF356LFX.bed.gz",
+        "mm9": stanford_url + "mm9-mouse/mm9-blacklist.bed.gz",
+        "mm10": stanford_url + "mm10-mouse/mm10.blacklist.bed.gz",
         # for testing purposes
         "this was a triumph": "I'm making a note here: 'Huge success'",
     }
@@ -43,6 +45,15 @@ class BlacklistPlugin(Plugin):
             sys.stderr.write(str(e))
             sys.stderr.write(f"Could not download blacklist file from {link}")
 
+        # convert UCSC format to Ensembl/NCBI format
+        if genome.name.split(".")[0] == "GRCh38":
+            os.rename(fname, fname + ".tmp")
+            with open(fname + ".tmp") as old_bed, open(fname, "w") as new_bed:
+                for line in old_bed:
+                    line = line[3:]  # remove "chr" from the start of each line
+                    new_bed.write(line)
+            os.unlink(fname + ".tmp")
+
     def get_properties(self, genome):
         props = {"blacklist": re.sub(".fa(.gz)?$", ".blacklist.bed", genome.filename)}
         return props
diff --git a/genomepy/utils.py b/genomepy/utils.py
index 723cc5f..ccefa68 100644
--- a/genomepy/utils.py
+++ b/genomepy/utils.py
@@ -226,7 +226,8 @@ def glob_ext_files(dirname, ext="fa"):
 def get_genomes_dir(genomes_dir=None, check_exist=True):
     """import genomes_dir if none is given, and check validity"""
     if not genomes_dir:
-        genomes_dir = config.get("genomes_dir", None)
+        # backwards compatibility for "genome_dir" (this fixes issue #87)
+        genomes_dir = config.get("genomes_dir", config.get("genome_dir", None))
     if not genomes_dir:
         raise exceptions.ConfigError("Please provide or configure a genomes_dir")
 
@@ -310,6 +311,7 @@ def gzip_and_name(fname, gzip_file=True):
         with open(fname, "rb") as f_in:
             with gzip.open(fname + ".gz", "wb") as f_out:
                 shutil.copyfileobj(f_in, f_out)
+        os.unlink(fname)
         fname += ".gz"
     return fname
 


Test patch:
diff --git a/tests/02_utils_test.py b/tests/02_utils_test.py
index 7c9c105..a8c359b 100644
--- a/tests/02_utils_test.py
+++ b/tests/02_utils_test.py
@@ -185,6 +185,7 @@ def test_gunzip_and_name(fname="tests/data/small_genome.fa.gz"):
     fname, gzip_file = genomepy.utils.gunzip_and_name(fname)
     assert gzip_file and fname.endswith(".fa")
     assert os.path.exists(fname)
+    assert not os.path.exists(fname + ".gz")
 
 
 def test_gzip_and_name(fname="tests/data/small_genome.fa"):
@@ -192,10 +193,12 @@ def test_gzip_and_name(fname="tests/data/small_genome.fa"):
     fname = genomepy.utils.gzip_and_name(fname)
     assert fname.endswith(".gz")
     assert os.path.exists(fname)
+    assert not os.path.exists(fname[:-3])
 
     fname, _ = genomepy.utils.gunzip_and_name(fname)
     assert fname.endswith(".fa")
     assert os.path.exists(fname)
+    assert not os.path.exists(fname + ".gz")
 
 
 def test_bgzip_and_name(fname="tests/data/small_genome.fa"):
@@ -203,6 +206,7 @@ def test_bgzip_and_name(fname="tests/data/small_genome.fa"):
     fname = genomepy.utils.bgzip_and_name(fname)
     assert fname.endswith(".gz")
     assert os.path.exists(fname)
+    assert not os.path.exists(fname[:-3])
 
     with pytest.raises(sp.CalledProcessError):
         genomepy.utils.bgzip_and_name("tests/data/nofile.fa")
diff --git a/tests/04_genome_test.py b/tests/04_genome_test.py
index 538d36d..68fd780 100644
--- a/tests/04_genome_test.py
+++ b/tests/04_genome_test.py
@@ -3,6 +3,7 @@ import os
 import pytest
 import shutil
 
+from stat import S_IREAD, S_IRGRP, S_IROTH
 from genomepy.provider import ProviderBase
 
 
@@ -164,7 +165,7 @@ def test__read_metadata(genome="tests/data/small_genome.fa.gz"):
     if os.path.exists(readme):
         os.unlink(readme)
     metadata = g._read_metadata()
-    assert metadata["provider"] == "Unknown"
+    assert metadata["provider"] == "na"
 
     # no overwrites to metadata
     with open(readme, "w") as f:
@@ -187,6 +188,17 @@ def test__read_metadata(genome="tests/data/small_genome.fa.gz"):
     assert metadata1["provider"] == "NCBI"
     metadata2, _ = genomepy.utils.read_readme(readme)
     assert metadata2["provider"] == "NCBI"
+
+    # no writing permission to file
+    with open(readme, "w") as f:
+        f.writelines(
+            "genome url: https://ftp.ncbi.nlm.nih.gov/genomes/all/GCF/000/"
+            "146/465/GCF_000146465.1_ASM14646v1/"
+            "GCF_000146465.1_ASM14646v1_genomic.fna.gz\n"
+        )
+    os.chmod(readme, S_IREAD | S_IRGRP | S_IROTH)
+    metadata1 = g._read_metadata()
+    assert metadata1["provider"] == "na"
     os.unlink(readme)
 
 
diff --git a/tests/11_plugins_test.py b/tests/11_plugins_test.py
index a900fd6..9687dfd 100644
--- a/tests/11_plugins_test.py
+++ b/tests/11_plugins_test.py
@@ -69,22 +69,19 @@ def genome(request):
 
 def test_blacklist(capsys, genome):
     """Create blacklist."""
+    # independent of bgzipping
+    # no need to check for both .fa and .fa.gz.
+    if genome.filename.endswith(".fa.gz"):
+        pass
+
     p = BlacklistPlugin()
+    fname = re.sub(".fa(.gz)?$", ".blacklist.bed", genome.filename)
 
     # no blacklist found
     genome.name = "ce01"
     p.after_genome_download(genome, force=True)
     captured = capsys.readouterr().err.strip()
     assert captured.endswith(f"No blacklist found for {genome.name}")
-    genome.name = "ce10"
-
-    # download
-    p.after_genome_download(genome, force=True)
-    fname = re.sub(".fa(.gz)?$", ".blacklist.bed", genome.filename)
-    assert os.path.exists(fname)
-
-    # don't overwrite
-    dont_overwrite(p, genome, fname)
 
     # error downloading blacklist
     genome.name = "this was a triumph"
@@ -93,14 +90,33 @@ def test_blacklist(capsys, genome):
     link = "I'm making a note here: 'Huge success'"
     assert captured.endswith(f"Could not download blacklist file from {link}")
 
-    # download
+    # download UCSC blacklist
     genome.name = "ce10"
     p.after_genome_download(genome, force=True)
-    fname = re.sub(".fa(.gz)?$", ".blacklist.bed", genome.filename)
+    captured = capsys.readouterr().err.strip()
+    assert captured.endswith("ce10-C.elegans/ce10-blacklist.bed.gz")
     assert os.path.exists(fname)
+    with open(fname) as blacklist:
+        for line in blacklist:
+            assert line.startswith("chr")
+            break
+    os.unlink(fname)
+
+    # download Ensembl/NCBI blacklist
+    genome.name = "GRCh38"
+    p.after_genome_download(genome, force=True)
+    captured = capsys.readouterr().err.strip()
+    assert captured.endswith("ENCFF356LFX/@@download/ENCFF356LFX.bed.gz")
+    with open(fname) as blacklist:
+        for line in blacklist:
+            assert not line.startswith("chr")
+            break
 
     # don't overwrite
     dont_overwrite(p, genome, fname)
+    os.unlink(fname)
+
+    genome.name = "ce10"
 
 
 def test_bowtie2(genome, threads=2):
@@ -168,7 +184,7 @@ def test_hisat2(capsys, genome, threads=2):
         assert os.path.exists(os.path.join(genome.genome_dir, "exon_sites.txt"))
         # check if annotation file is still the same
         assert os.path.exists(genome.annotation_gtf_file)
-        assert genome.annotation_gtf_file.endswith(".gtf")
+        assert genome.annotation_gtf_file.endswith(".gtf.gz")
     else:
         assert captured == "Creating Hisat2 index without annotation file."
 
@@ -208,7 +224,7 @@ def test_star(capsys, genome, threads=2):
         assert captured == ""
         # check if annotation file is still the same
         assert os.path.exists(genome.annotation_gtf_file)
-        assert genome.annotation_gtf_file.endswith(".gtf")
+        assert genome.annotation_gtf_file.endswith(".gtf.gz")
     else:
         assert captured == "Creating STAR index without annotation file."