Skip to content

Commit

Permalink
Merge branch 'master' of https://github.com/ged-lab/khmer into fix/787
Browse files Browse the repository at this point in the history
  • Loading branch information
aditi9783 committed Feb 23, 2015
2 parents 257cb3d + 662276c commit ba4e0cd
Show file tree
Hide file tree
Showing 24 changed files with 303 additions and 87 deletions.
41 changes: 41 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,44 @@
2015-02-23 Kevin Murray <spam@kdmurray.id.au>

* khmer/load_pe.py: Remove unused/undocumented module. See #784

2015-02-21 Hussien Alameldin <hussien@msu.edu>

* sandbox/calc-best-assembly.py, collect-variants.py, graph-size.py: Set executable bits using "chmod +x"

2015-02-21 Michael R. Crusoe <mcrusoe@msu.edu>

* khmer/_khmermodule.cc,lib/read_parsers.cc: Rename the 'accuracy' attribute
of ReadParser Reads to 'quality'
* tests/test_read_parsers.py: update test to match

2015-02-21 Rhys Kidd <rhyskidd@gmail.com>

* sandbox/{calc-best-assembly,calc-error-profile,normalize-by-align,
read_aligner,slice-reads-by-coverage}.py: reference /usr/bin/env python2
in the #! line.

2015-02-21 Rhys Kidd <rhyskidd@gmail.com>

* sandbox/sweep-paired-reads.py: remove empty script

2015-02-20 Titus Brown <titus@idyll.org>

* doc/dev/scripts-and-sandbox.txt: policies for sandbox/ and scripts/
content, and a process for adding new command line scripts into scripts/.
* doc/dev/index.txt: added scripts-and-sandbox to developer doc index.

2015-02-20 Michael R. Crusoe <mcrusoe@msu.edu>

* khmer/_khmermodule.cc: convert C++ out of memory exceptions to Python
out of memory exception.
* test/test_{counting_hash,counting_single,hashbits_obj,labelhash,
scripts}.py: partial tests for the above

2015-02-20 Aditi Gupta <agupta@msu.edu>

* doc/dev/coding-guidelines-and-review.txt: fixed spelling errors.

2015-02-19 Michael R. Crusoe <mcrusoe@msu.edu>

* doc/dev/coding-guidelines-and-review.txt: added checklist for new CPython
Expand Down
10 changes: 8 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ CPPCHECK=ls lib/*.cc khmer/_khmermodule.cc | grep -v test | cppcheck -DNDEBUG \
--quiet -Ilib -Ithird-party/bzip2 -Ithird-party/zlib \
-Ithird-party/smhasher

UNAME := $(shell uname)
ifeq ($(UNAME),Linux)
TESTATTR='!known_failing,!jenkins'
else
TESTATTR='!known_failing,!jenkins,!linux'
endif

## all : default task; compile C++ code, build shared object library
all: sharedobj
Expand Down Expand Up @@ -162,7 +168,7 @@ diff-cover.html: coverage-gcovr.xml coverage.xml
--html-report diff-cover.html

nosetests.xml: FORCE
./setup.py nosetests --with-xunit
./setup.py nosetests --with-xunit --attr ${TESTATTR}

## doxygen : generate documentation of the C++ and Python code
doxygen: doc/doxygen/html/index.html
Expand All @@ -180,7 +186,7 @@ lib:
## test : run the khmer test suite
test: FORCE
./setup.py develop
./setup.py nosetests
./setup.py nosetests --attr ${TESTATTR}

sloccount.sc: ${CPPSOURCES} ${PYSOURCES} $(wildcard tests/*.py) Makefile
sloccount --duplicates --wide --details lib khmer scripts tests \
Expand Down
4 changes: 2 additions & 2 deletions doc/dev/coding-guidelines-and-review.txt
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ Checklist
Copy and paste the following into a pull request comment when it is
ready for review::

- [ ] Is it mergable?
- [ ] Is it mergeable?
- [ ] Did it pass the tests?
- [ ] If it introduces new functionality in scripts/ is it tested?
Check for code coverage with `make clean diff-cover`
Expand All @@ -80,7 +80,7 @@ in the middle.
CPython Checklist
-----------------

Here's a checklist for new CPython types with futureproofing for Python 3::
Here's a checklist for new CPython types with future-proofing for Python 3::

- [ ] the CPython object name is of the form `khmer_${OBJECTNAME}_Object`
- [ ] Named struct with `PyObject_HEAD` macro
Expand Down
3 changes: 2 additions & 1 deletion doc/dev/index.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@ Contents:
.. toctree::
:maxdepth: 1

CODE_OF_CONDUCT
getting-started
a-quick-guide-to-testing
codebase-guide
coding-guidelines-and-review
scripts-and-sandbox
for-khmer-developers
release

details
development
crazy-ideas
CODE_OF_CONDUCT
115 changes: 115 additions & 0 deletions doc/dev/scripts-and-sandbox.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
Command line scripts, ``scripts/``, and ``sandbox/``
====================================================

.. note::

This document applies through khmer/oxli 2.0/3.0 (see
:doc:`../roadmap`) - we will revisit when the Python API falls
under semantic versioning for oxli 4.0.

khmer has two conflicting goals: first, we want to provide a reliable
piece of software to our users; and second, we want to be flexible and
enable exploration of new algorithms and programs. To this end,
we've split our command line scripts across two directories,
``scripts/`` and ``sandbox/``. The former is the staid, boring, reliable
code; the latter is a place for exploration.

As a result, we are committed to high test coverage, stringent code
review, and `Semantic Versioning <http://semver.org/>`__ for files in
``scripts/``, but explicitly *not* committed to this for files and
functionality implemented in ``sandbox/``. So, putting a file into
``scripts/`` is a big deal, especially since it increases our maintenance
burden for the indefinite future.

We've roughed out the following process for moving scripts into ``scripts/``:

* Command line scripts start in ``sandbox/``;
* Once their utility is proven (in a paper, for example), we can propose to
move them into ``scripts/``;
* There's a procedure for moving scripts from ``sandbox/`` into ``scripts/``.

Read on!

Sandbox script requirements and suggestions
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

All scripts in ``sandbox/`` must:

* be importable (enforced by ``test_import_all`` in
``test_sandbox_scripts.py``)
* be mentioned in ``sandbox/README.rst``
* have a hash-bang line (``#! /usr/bin/env python2``) at the top
* be command-line executable (``chmod a+x``)
* have a Copyright message (see below)

All *new* scripts being added to ``sandbox/`` should:

* have decent automated tests
* be used in a protocol (see khmer-protocols) or a recipe (see khmer-recipes)
* be pep8 clean and pylint clean-ish (see ``make pep8`` and ``make_diff_pylint``).

Command line standard options for scripts/
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

All scripts in scripts/ should have the following options, if they could apply:

* ``--version`` - should always apply
* ``--help`` - should always apply
* ``--force`` - override any sanity checks that may prevent the script from running
* ``--loadtable`` and ``--savetable`` - where appropriate (see khmer_args.py)

Copyright message
~~~~~~~~~~~~~~~~~

Our current Copyright message is::

#
# This file is part of khmer, http://github.com/ged-lab/khmer/, and is
# Copyright (C) Michigan State University, 2009-2015. It is licensed under
# the three-clause BSD license; see doc/LICENSE.txt.
# Contact: khmer-project@idyll.org
#

The beginning year should be the first year that this file existed in
the repo; the end year should be the last year a coding change was
made in the file.

Upgrading a script from 'sandbox' to 'scripts'
----------------------------------------------

First, everything needed (all library support code) should be already
committed to khmer master after the usual review process; the relevant
script(s) should be in ``sandbox/``.

Second, an issue should be started explicitly to discuss whether the
script(s) should be moved from ``sandbox/`` into ``scripts/``. This issue
should discuss the general need for this script, outside of a particular
paper pipeline. (Note that there is no imperative to move a script
out of ``sandbox/``; if we think it's useful code to have around and
want to keep it functioning, we should just add in automated tests and
otherwise level it up.)

Third, assuming we reach general agreement about moving the script(s)
into ``scripts/``, start a pull request to do so, referencing the
issue and containing the following checklist. The PR should start by
moving the script from ``sandbox/`` into ``scripts/``, and moving the
tests out of the ``test_sandbox_scripts.py`` file.

Last but not least, intensive code review may raise more general
issues that could apply to the entire code base; if contentious or
needing discussion, these issues may be punted to general issues so as
to not block a merge.

A checklist for moving a script into the scripts/ directory from sandbox/
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Copy or paste this checklist into the PR, in addition to the normal
development/PR checklist::

- [ ] most or all lines of code are covered by automated tests (see output of ``make diff-cover``)
- [ ] ``make diff_pylint`` is clean
- [ ] the script has been updated with a ``get_parser()`` and added to doc/user/scripts.txt
- [ ] argparse help text exists, with an epilog docstring, with examples and options
- [ ] standard command line options are implemented
- [ ] version and citation information is output to STDERR (`khmer_args.info(...)`)
- [ ] runtime diagnostic information (progress, etc.) is output to STDERR
Loading

0 comments on commit ba4e0cd

Please sign in to comment.