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

--quiet expecting an argument is a breaking change #1660

Closed
victorlin opened this issue May 16, 2022 · 12 comments
Closed

--quiet expecting an argument is a breaking change #1660

victorlin opened this issue May 16, 2022 · 12 comments
Labels
bug Something isn't working

Comments

@victorlin
Copy link

victorlin commented May 16, 2022

Snakemake version
7.7.0

Describe the bug

The new version 7.7.0 breaks scripts that use --quiet with a target name following the flag (e.g. snakemake --cores 1 --quiet myrule).

Previously, with version 7.6.2, --quiet was a standalone boolean flag:

usage: snakemake [-h] [--dry-run] [--profile PROFILE] [--cache [RULE ...]] [--snakefile FILE] [--cores [N]] [--jobs [N]]
                 ...
                 [--quiet] [--print-compilation]
                 ...
                 [target ...]

Now, with a change in --quiet behavior introduced in c8d81d0 and released in version 7.7.0, it will try to read any arguments:

usage: snakemake [-h] [--dry-run] [--profile PROFILE] [--cache [RULE ...]] [--snakefile FILE] [--cores [N]] [--jobs [N]]
                 ...
                 [--quiet [{progress,rules,all} ...]]
                 ...
                 [target ...]

Logs

See example below.

Minimal example

Setup:

cat >Snakefile << ~~
rule myrule:
    shell: "echo \"Hello world\""
~~
pip install snakemake==7.6.2
snakemake --cores 1 --quiet myrule
# Job stats:
# ...
# Hello world
pip install snakemake==7.7.0
snakemake --cores 1 --quiet myrule
# usage: snakemake [-h] [--dry-run] [--profile PROFILE] [--cache [RULE ...]] [--snakefile FILE] [--cores [N]] [--jobs [N]]
# ...
# snakemake: error: argument --quiet/-q: invalid choice: 'myrule' (choose from 'progress', 'rules', 'all')

Workarounds

Note that this only breaks when the target is specified directly following --quiet. So these still work with 7.7.0:

snakemake --cores 1 myrule --quiet
snakemake --quiet --cores 1 myrule
snakemake --cores 1 --quiet -- myrule

Additional context

N/A

@dariober
Copy link
Contributor

dariober commented May 17, 2022

If it helps, this should work but I haven't tried it:

snakemake --cores 1 --quiet -- myrule

The -- separates the list of optional arguments from the list of positional arguments. It's fairly standard behavior of command line parsers so I wouldn't consider it a bug even if it breaks backward compatibility (e.g. to grep --foo in a file you would do grep -- --foo my.file).

@victorlin
Copy link
Author

@dariober thanks, that is a good practice to follow that I wasn't aware of until now. I've added your working example to the list in the issue description.

I agree that this isn't really a bug and there is no easy fix. Just having this here for anyone else who might come across the same thing, especially since the change in --quiet was not added to 7.7.0 release notes so it took some digging to understand.

@jdblischak
Copy link
Contributor

I just got bit by this change in behavior, and I was also surprised it wasn't included in the changelog.

The -- workaround is sufficient for the minimal example in this Issue, but I'm not getting the expected behavior when I use it with my own Snakefiles (ie it's still printing all the rules despite passing --quiet). I'll try to create a minimal example to better pinpoint the problem.

@jdblischak
Copy link
Contributor

I think I figured it out. I can't use the -- workaround when I also define command-line arguments via a profile.

# using the same Snakefile above
mkdir profile
echo "cores: 1" > profile/config.yaml

# works
snakemake --cores 1 --quiet -- myrule

# works
snakemake --profile profile/ myrule

# fails
snakemake --profile profile/ --quiet -- myrule

Error: you need to specify the maximum number of CPU cores to be used at the same time. If you want to use N cores, say --cores N or -cN. For all cores on your system (be sure that this is appropriate) use --cores all. For no parallelization use --cores 1 or -c1.

# fails
snakemake --profile profile/ --quiet all myrule

snakemake: error: argument --quiet/-q: invalid choice: 'myrule' (choose from 'progress', 'rules', 'all')

# works
snakemake --quiet --profile profile/ myrule

Essentially, it appears that now I have to specify --quiet prior to --profile. This is inconvenient, unexpected, and arbitrary. Am I missing something obvious?

@jdblischak
Copy link
Contributor

Another strange behavior I noticed. The default behavior of --quiet is affected by the use of --profile. Again, using the same Snakefile and profile/config.yaml as above:

# 7.6.2

# expected
$ snakemake -nq
Job stats:
job       count    min threads    max threads
------  -------  -------------  -------------
myrule        1              1              1
total         1              1              1

# expected - same as above
$ snakemake --profile profile/ -nq
Job stats:
job       count    min threads    max threads
------  -------  -------------  -------------
myrule        1              1              1
total         1              1              1

# 7.7.0

# expected
$ snakemake -nq
Building DAG of jobs...
Job stats:
job       count    min threads    max threads
------  -------  -------------  -------------
myrule        1              1              1
total         1              1              1

# unexpected! Why is the rule shown?
$ snakemake --profile profile/ -nq
Building DAG of jobs...
Job stats:
job       count    min threads    max threads
------  -------  -------------  -------------
myrule        1              1              1
total         1              1              1


[Mon May 23 19:14:46 2022]
rule myrule:
    jobid: 0
    resources: tmpdir=/local/38306425

Job stats:
job       count    min threads    max threads
------  -------  -------------  -------------
myrule        1              1              1
total         1              1              1

This was a dry-run (flag -n). The order of jobs does not reflect the order of execution.

# expected - explicitly quieting the rules still works
$ snakemake --profile profile/ -nq rules
Building DAG of jobs...
Job stats:
job       count    min threads    max threads
------  -------  -------------  -------------
myrule        1              1              1
total         1              1              1

@pvandyken
Copy link
Contributor

@jdblischak The bug preventing the mixing of --profile and --quiet is fixed with #1764.

@jdblischak
Copy link
Contributor

@pvandyken Thanks for fixing the bug! I confirmed it is fixed in 7.9.0:

snakemake --version
## 7.9.0

cat > Snakefile << EOF
rule myrule:
    shell: "echo \"Hello world\""
EOF

mkdir profile
echo "cores: 1" > profile/config.yaml

# all of the following produce the same output
snakemake -nq
snakemake --profile profile/ -nq
snakemake --profile profile/ -nq rules
snakemake -nq --profile profile/
snakemake -nq rules --profile profile/

## Building DAG of jobs...
## Job stats:
## job       count    min threads    max threads
## ------  -------  -------------  -------------
## myrule        1              1              1
## total         1              1              1

@jdblischak
Copy link
Contributor

@victorlin Thanks for reporting this bug. I think you can close the Issue now

@jdblischak
Copy link
Contributor

Actually, looking back at the original post, I realize the issue was the change in behavior of the --quiet flag accepting arguments. This still requires the -- workaround (or specifying --quiet after the target), and I agree could be better documented since the --quiet flag has been around a long time as a standalone flag.

% snakemake --cores 1 --quiet myrule
snakemake: error: argument --quiet/-q: invalid choice: 'myrule' (choose from 'progress', 'rules', 'all')

% snakemake --cores 1 --quiet -- myrule
Building DAG of jobs...
Using shell: /usr/bin/bash
Provided cores: 1 (use --cores to define parallelism)
Rules claiming more threads will be scaled down.
Job stats:
job       count    min threads    max threads
------  -------  -------------  -------------
myrule        1              1              1
total         1              1              1

Select jobs to execute...
Hello world
Complete log: .snakemake/log/2022-07-19T121336.050016.snakemake.log

% snakemake --cores 1 myrule --quiet
Building DAG of jobs...
Using shell: /usr/bin/bash
Provided cores: 1 (use --cores to define parallelism)
Rules claiming more threads will be scaled down.
Job stats:
job       count    min threads    max threads
------  -------  -------------  -------------
myrule        1              1              1
total         1              1              1

Select jobs to execute...
Hello world
Complete log: .snakemake/log/2022-07-19T121647.506241.snakemake.log

@victorlin It's your call as far as closing this Issue or leaving it open

@victorlin
Copy link
Author

victorlin commented Jul 19, 2022

I think it's safe to close this as "won't fix", since there is an easy workaround and I doubt --quiet will be changed back to a standalone boolean flag.

@victorlin victorlin closed this as not planned Won't fix, can't repro, duplicate, stale Jul 19, 2022
@pvandyken
Copy link
Contributor

pvandyken commented Jul 19, 2022

I think I figured it out. I can't use the -- workaround when I also define command-line arguments via a profile.

@jdblischak Is this still an issue?

@jdblischak
Copy link
Contributor

Is this still an issue?

@pvandyken No, your PR fixed that issue. That is what I confirmed above in #1660 (comment)

My comments after that were related to the subject of @victorlin's original post in this Issue, which is the fact that the ordering of --quiet and a target require more care now that --quiet can accept arguments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants