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

misc small fixes #877

Closed
wants to merge 11 commits into from
Closed

misc small fixes #877

wants to merge 11 commits into from

Conversation

notestaff
Copy link
Contributor

Misc small fixes. Factoring out parts of #854 not specific to kmer operations.

  • added gitignore for emacs backup files
  • added printing of defaults to help msgs. fixed log message that always reported an exception when keeping tmp files, even when there was none. added parse_cmd() and run_cmd() utils to cmd.py .
  • added developer docs on adding top-level scripts
  • fixed comments; added uncompressed_file_type() to util/file.py; improved preservation of tmp files on request
  • added misc utils: as_type, subdict, chk
  • reduced code duplication in .travis.yml
  • added pytest option to mark tests as slow and run them only when --runslow is specified

@notestaff
Copy link
Contributor Author

Would it be better to break out these changes into separate PRs? Some are general fixes, some are general utils on which #854 relies.


Args:
Copy link
Member

Choose a reason for hiding this comment

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

What led to these changes, or what do they fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are useful for testing, if you want to run a command but also see the results of parsing its arguments, e.g. https://github.com/broadinstitute/viral-ngs/pull/854/files#diff-771ac3c0ea113ca9a6634523f4fb6893R273
Also calling run_cmd() is clearer than the current pattern of calling the parser manually then calling its func_main .

@@ -11,6 +11,7 @@ env:
- PIP_DIR="$HOME/virtualenv"
- GATK_PATH="$CACHE_DIR"
- PYTHONIOENCODING=UTF8
- PYTEST_ADDOPTS="-rsxX -n 2 --durations=25 --fixture-durations=10 --junit-xml=pytest.xml --cov-report= --cov broad_utils --cov illumina --cov assembly --cov interhost --cov intrahost --cov metagenomics --cov ncbi --cov read_utils --cov reports --cov taxon_filter --cov tools --cov util --cov file_utils"
Copy link
Member

Choose a reason for hiding this comment

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

I had moved these travis env variables to the parts of the build matrix that actually use py.test... there are a bunch of other points in the build matrix that have nothing to do with pytest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right; on the other hand, there are parts that don't use GATK_PATH either. There's a downside to having copies of the same complex line in several places -- the copies have to be kept in sync.
But if having PYTEST_ADDOPTS defined in jobs that don't use it might cause problems, I'll revert this change.

@notestaff notestaff closed this Nov 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants