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

EXP: update sig manifest default rebuilding behavior for v5. #3074

Open
wants to merge 4 commits into
base: start_v5
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/sourmash/cli/sig/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,29 @@ def subparser(subparsers):
"-f", "--force", action="store_true", help="try to load all files as signatures"
)
subparser.add_argument(
"--no-rebuild-manifest",
"--rebuild-manifest",
help="use existing manifest if available",
action="store_true",
default=None,
)
subparser.add_argument(
"--no-rebuild-manifest",
help="force rebuilding manifest if available",
action="store_false",
dest="rebuild_manifest",
)

subparser.add_argument(
"-F",
"--manifest-format",
help="format of manifest output file; default is 'csv')",
default="csv",
choices=["csv", "sql"],
)
subparser.add_argument(
"--v4", dest="cli_version", action="store_const", const="v4", default="v4"
)
subparser.add_argument("--v5", dest="cli_version", action="store_const", const="v5")


def main(args):
Expand Down
20 changes: 15 additions & 5 deletions src/sourmash/sig/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,22 @@
error("Use -d/--debug for details.")
sys.exit(-1)

rebuild = True
if args.no_rebuild_manifest:
debug("sig manifest: not forcing rebuild.")
# behavior switch: in v4, manifests were rebuilt by default; in v5, not.
if args.cli_version == "v4":
rebuild = True

# was --no-rebuild-manifest specified?
if args.rebuild_manifest is False:
debug("sig manifest: not forcing rebuild.")
rebuild = False
else:
# either left as default (None) or set (True) - rebuild
debug("sig manifest: forcing rebuild.")
else: # args.cli_version == 'v5':
rebuild = False
else:
debug("sig manifest: forcing rebuild.")
if args.rebuild_manifest:
debug("sig manifest: forcing rebuild.")
rebuild = True

Check warning on line 378 in src/sourmash/sig/__main__.py

View check run for this annotation

Codecov / codecov/patch

src/sourmash/sig/__main__.py#L377-L378

Added lines #L377 - L378 were not covered by tests

manifest = sourmash_args.get_manifest(loader, require=True, rebuild=rebuild)

Expand Down
37 changes: 30 additions & 7 deletions tests/test_cmd_signature.py
Original file line number Diff line number Diff line change
Expand Up @@ -3952,7 +3952,7 @@ def test_sig_manifest_1_zipfile_csv_gz(runtmp):


def test_sig_manifest_1_zipfile_already_exists(runtmp):
# make a manifest from a .zip file; f
# make a manifest from a .zip file;
protzip = utils.get_test_data("prot/protein.zip")

mf_csv = runtmp.output("mf.csv")
Expand Down Expand Up @@ -4098,10 +4098,16 @@ def test_sig_manifest_does_not_exist(runtmp):
)


def test_sig_manifest_7_allzip_1(runtmp):
def test_sig_manifest_7_allzip_1(runtmp, cli_v4_only):
# the rebuilt manifest w/o '-f' will miss dna-sig.noext
# note: default in v4 is to rebuild manifest
allzip = utils.get_test_data("prot/all.zip")
runtmp.sourmash("sig", "manifest", allzip, "-o", "xyz.csv")
runtmp.sourmash(
"sig", "manifest", allzip, "-o", "xyz.csv", "-d", version=cli_v4_only
)

print(runtmp.last_result.out)
print(runtmp.last_result.err)

manifest_fn = runtmp.output("xyz.csv")
with open(manifest_fn, newline="") as csvfp:
Expand All @@ -4126,11 +4132,28 @@ def test_sig_manifest_7_allzip_2(runtmp):
assert "dna-sig.noext" in filenames


def test_sig_manifest_7_allzip_3(runtmp):
# the existing manifest contains 'dna-sig.noext' whther or not -f is
# used.
def test_sig_manifest_7_allzip_3_no_rebuild(runtmp, cli_v4_only):
# the manifest contains 8 entries.
# note: --no-rebuild-manifest is default behavior on v4, but not on v5.
allzip = utils.get_test_data("prot/all.zip")
runtmp.sourmash(
"sig", "manifest", allzip, "-o", "xyz.csv", "--no-rebuild", version=cli_v4_only
)

manifest_fn = runtmp.output("xyz.csv")
with open(manifest_fn, newline="") as csvfp:
manifest = CollectionManifest.load_from_csv(csvfp)

assert len(manifest) == 8
filenames = set(row["internal_location"] for row in manifest.rows)
assert "dna-sig.noext" in filenames


def test_sig_manifest_7_allzip_3_no_rebuild_is_default_v5(runtmp, cli_v5_only):
# by default, do not rebuild the manifest.
# note: --no-rebuild-manifest is on by default in v5
allzip = utils.get_test_data("prot/all.zip")
runtmp.sourmash("sig", "manifest", allzip, "-o", "xyz.csv", "--no-rebuild")
runtmp.sourmash("sig", "manifest", allzip, "-o", "xyz.csv", version=cli_v5_only)

manifest_fn = runtmp.output("xyz.csv")
with open(manifest_fn, newline="") as csvfp:
Expand Down
Loading