Repository: kblin/ncbi-genome-download

Issue:
metadata option unworking
# version of ncbi-genome-download 
`0.2.11`

# command
```
python3 ncbi-genome-download/ncbi-genome-download-runner.py --verbose --genus "Kluyvera" --metadata
-table data.tsv --format fasta -p 10 --retries 1 bacteria
```

# result

All genomes are downloaded but the data.tsv file doesnt contains the metadata, only the header.


```
cat data.tsv 

assembly_accession	bioproject	biosample	wgs_master	excluded_from_refseq	refseq_category	relation_to_type_material	taxid	species_taxid	organism_name	infraspecific_name	isolate	version_status	assembly_level	release_type	genome_rep	seq_rel_date	asm_name	submitter	gbrs_paired_asm	paired_asm_comp	ftp_path	local_filename
```

Discussion:
kblin: Hi there, thanks for the patch! I'll try to make some time to look at it this week.

Patch:
diff --git a/ncbi_genome_download/core.py b/ncbi_genome_download/core.py
index 69efc2b..c99fd6b 100644
--- a/ncbi_genome_download/core.py
+++ b/ncbi_genome_download/core.py
@@ -192,15 +192,19 @@ def config_download(config):
 
         if config.parallel == 1:
             for entry, group in download_candidates:
-                download_jobs.extend(create_downloadjob(entry, group, config))
+                curr_jobs = create_downloadjob(entry, group, config)
+                fill_metadata(curr_jobs, entry)
+                download_jobs.extend(curr_jobs)
+
             for dl_job in download_jobs:
                 worker(dl_job)
         else:  # pragma: no cover
             # Testing multiprocessing code is annoying
             pool = Pool(processes=config.parallel)
 
-            for created_dl_job in pool.imap_unordered(downloadjob_creator_caller, [ (entry, group, config) for entry, group in download_candidates ]):
+            for index, created_dl_job in enumerate(pool.imap(downloadjob_creator_caller, [ (entry, group, config) for entry, group in download_candidates ])):
                 download_jobs.extend(created_dl_job)
+                fill_metadata(created_dl_job, download_candidates[index][0])#index is conserved from download_candidates with the use of imap
 
             jobs = pool.map_async(worker, download_jobs)
             try:
@@ -222,6 +226,11 @@ def config_download(config):
         return 75
     return 0
 
+def fill_metadata(jobs, entry):
+    for job in jobs:
+        if job.full_url is not None:#if it is None, it's a symlink making, so nothing to write
+            mtable = metadata.get()
+            mtable.add(entry, job.local_file)
 
 def select_candidates(config):
     """Select candidates to download.
@@ -534,10 +543,6 @@ def download_file_job(entry, directory, checksums, filetype='genbank', symlink_p
     if symlink_path is not None:
         full_symlink = os.path.join(symlink_path, filename)
 
-    # Keep metadata around
-    mtable = metadata.get()
-    mtable.add(entry, local_file)
-
     return DownloadJob(full_url, local_file, expected_checksum, full_symlink)
 # pylint: enable=too-many-arguments,too-many-locals
 


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index dda0f74..4210c57 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -4,12 +4,13 @@ from argparse import Namespace
 from collections import OrderedDict
 import os
 from os import path
+from multiprocessing import Pool
 
 import pytest
 import requests_mock
 from requests.exceptions import ConnectionError
 
-from ncbi_genome_download import core
+from ncbi_genome_download import core, metadata
 from ncbi_genome_download import NgdConfig, SUPPORTED_TAXONOMIC_GROUPS
 
 
@@ -132,6 +133,28 @@ def test_download_metadata(monkeypatch, mocker, req, tmpdir):
     assert core.create_downloadjob.call_count == 4
     assert metadata_file.check()
 
+def test_metadata_fill(req, tmpdir):
+    entry, config, _ = prepare_create_downloadjob(req, tmpdir)
+    metadata.clear()#clear it, otherwise operations realized in other tests might impact it
+    assert len(core.metadata.get().rows) == 0
+    jobs = core.create_downloadjob(entry, 'bacteria', config)
+    core.fill_metadata(jobs, entry)
+    assert len(core.metadata.get().rows) == 1
+
+def test_metadata_fill_multi(req, tmpdir):
+    entry, config, joblist = prepare_create_downloadjob(req, tmpdir)
+    metadata.clear()#clear it, otherwise operations realized in other tests might impact it
+    jobs = []
+    assert len(core.metadata.get().rows) == 0
+    download_candidates = [(entry, 'bacteria')]
+    p = Pool(processes=1)
+    for index, created_dl_job in enumerate(p.imap(core.downloadjob_creator_caller, [ (curr_entry, curr_group, config) for curr_entry, curr_group in download_candidates ])):
+        jobs.extend(created_dl_job)
+        assert download_candidates[index][0] == entry
+        core.fill_metadata(created_dl_job, download_candidates[index][0])
+    expected = [j for j in joblist if j.local_file.endswith('_genomic.gbff.gz')]
+    assert len(core.metadata.get().rows) == 1
+    assert jobs == expected
 
 def test_download_complete(monkeypatch, mocker, req):
     summary_contents = open(_get_file('assembly_status.txt'), 'r').read()