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

Small Python cleanups. Everything passes flake8 now. #1

Merged
merged 1 commit into from Feb 13, 2022

Conversation

terrycojones
Copy link
Contributor

Hi Jesse. Here are some very small suggested Python clean-ups for you, if you'd like them. They're all trivial. I don't use snakemake but I think the import snakemake is correct. I added it because it stops tools like flake8 from complaining about the unknown name and also editors from flagging it as unknown.

Copy link
Owner

@jbloom jbloom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@terrycojones, thanks! I had one question: what does E option mean in set -Eeuo pipefail in bash? I could not find an explanation for this one by Google-ing.

@terrycojones
Copy link
Contributor Author

The -E is a bit obscure I guess. It's like set -e but causes a failure if an error is encountered in code executed in trap. Of course you don't have any traps, but you also don't have any pipes, so pipefail also isn't necessary. The -E is mentioned on the page your script points to https://vaneyckt.io/posts/safer_bash_scripts_with_set_euxo_pipefail/ (which I read years ago, after tracking down a nasty problem that was due to using -e but not realizing that it for some reason didn't apply in pipes). After that I got myself into the habit of always sticking set -Eeuo pipefail at the top of all bash scripts.

@terrycojones
Copy link
Contributor Author

Actually, man bash has a short and clear explanation: -E If set, any trap on ERR is inherited by shell functions, command substitutions, and commands executed in a subshell environment. The ERR trap is normally not inherited in such cases.

@jbloom jbloom merged commit c7a9a67 into jbloom:main Feb 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants