Address CR

DEVELOP.md

@@ -102,7 +102,7 @@ gulp prod
 
 ### 7. Run RabbitMQ (and keep it running)
 
-To test the workers locally, you'll need to install and run rabbitmq 
+To test the workers locally, you'll need to install and run rabbitmq
 during development. For example, on Mac OS X, you can do this via:
 
 ```
@@ -139,9 +139,9 @@ To run tests:
 
 ```
 source path/to/bin/activate
-source ./tests/ENV.sh    # make sure all our environment variables are around
-nosetests tests/python   # Run Python tests
-npm test                 # Run JS tests
+source ./tests/ENV.sh           # make sure all our environment variables are around
+./scripts/run-python-tests.sh   # Run Python tests
+./scripts/run-js-tests.sh       # Run JS tests
 ```
 
 To run an individual JavaScript test, you can use:

common/helpers.py

@@ -66,7 +66,7 @@ def find(iterable, pred):
             return x
 
 
-def from_epoch(dt):
+def to_epoch(dt):
     """Return the epoch time representation of a datetime object, `dt`."""
     epoch = datetime(1970, 1, 1, tzinfo=dt.tzinfo)
     return (dt - epoch).total_seconds()

cycledash/comments.py

@@ -4,7 +4,7 @@
 from flask.ext.restful import abort, Resource, fields
 from sqlalchemy import select, func, desc
 
-from common.helpers import tables, from_epoch
+from common.helpers import tables, to_epoch
 from cycledash import db
 from cycledash.helpers import (prepare_request_data, success_response,
                                validate_with, abort_if_none_for, EpochField,
@@ -105,7 +105,7 @@ def get_vcf_comments(vcf_id):
 
     {
       "comments": {
-        "<row_key STRING>": [<comment>, ...],
+        "<row_key STRING>": [<comment_fields>, ...],
         "<row_key STRING>": [...], ...
       }
     }
@@ -121,8 +121,8 @@ def _row_key(comment, table):
         results_map = defaultdict(list)
         for comment in (dict(c) for c in results):
             row_key = _row_key(comment, user_comments)
-            comment['last_modified'] = from_epoch(comment['last_modified'])
-            comment['created'] = from_epoch(comment['created'])
+            comment['last_modified'] = to_epoch(comment['last_modified'])
+            comment['created'] = to_epoch(comment['created'])
             comment = camelcase_dict(comment)
             results_map[row_key].append(comment)
     return {'comments': results_map}
@@ -141,16 +141,16 @@ def get_last_comments(n=5):
 def epochify_comments(comments):
     """Sets `lastModified` and `created` to be epoch time instead of iso8061."""
     for c in comments:
-        c['lastModified'] = from_epoch(c['lastModified'])
-        c['created'] = from_epoch(c['created'])
+        c['lastModified'] = to_epoch(c['lastModified'])
+        c['created'] = to_epoch(c['created'])
     return comments
 
 
 def _ensure_not_out_of_date(comment, last_modified):
     """Assert that the comment has the same last_modified time,
     otherwise abort(409).
     """
-    current_time = from_epoch(comment['last_modified'])
+    current_time = to_epoch(comment['last_modified'])
     if current_time != last_modified:
         abort(409, message=('Comment id={} is out of date.'
                             .format(comment['id'])),

cycledash/genotypes.py

@@ -5,7 +5,7 @@
 
 from flask import request
 import flask.ext.restful as restful
-from flask.ext.restful import abort, Resource, fields, marshal_with
+from flask.ext.restful import Resource
 from sqlalchemy import (select, func, types, cast, join, outerjoin, asc, desc,
                         and_, Integer, Float, String)
 from sqlalchemy.sql import text

cycledash/helpers.py

@@ -5,7 +5,7 @@
 import re
 
 from cycledash import db
-from common.helpers import tables, from_epoch
+from common.helpers import tables, to_epoch
 
 from flask import jsonify, request
 import flask.ext.restful, flask.ext.restful.fields
@@ -20,6 +20,8 @@
 def underscorize(value):
     """Returns underscored version of a camelCase string.
 
+    e.g. camelCase => camel_case
+
     Raises ValueError if a value other than a string is passed.
     """
     res = RE_CAMELCASE_1.sub(r'_\1', value)
@@ -40,15 +42,16 @@ def camelcase(value):
 
 def underscorize_dict(value):
     """Return a dictionary with all keys and sub-keys underscorized.
+
+    e.g. camelCase => camel_case
     """
     return {underscorize(key): underscorize_dict(val)
             if isinstance(val, dict) else val
             for key, val in value.iteritems()}
 
 
 def camelcase_dict(value):
-    """Return a dictionary with all keys and sub-keys camelCased.
-    """
+    """Return a dictionary with all keys and sub-keys camelCased."""
     if isinstance(value, list):
         return [camelcase_dict(o) for o in value]
     elif isinstance(value, dict):
@@ -161,7 +164,7 @@ def request_wants_json():
 
 
 def marshal_with(fields, envelope=None):
-    """Wraps flast-restful's marshal_with to transform the returned object to
+    """Wraps flask-restful's marshal_with to transform the returned object to
     have camelCased keys."""
     def decorator(f):
         @functools.wraps(f)
@@ -218,4 +221,4 @@ class CollisionError(Exception):
 
 class EpochField(flask.ext.restful.fields.Raw):
     def format(self, value):
-        return from_epoch(value)
+        return to_epoch(value)

cycledash/runs.py

@@ -24,14 +24,9 @@
     'notes': fields.String,
     'vcf_header': fields.String,
 
-    # TODO: change to show these instead of *_id
-    # 'project': fields.Raw,
-    # 'normal_bam': fields.Raw,
-    # 'tumor_bam': fields.Raw,
-
-    'project_id': fields.Raw,
-    'normal_bam_id': fields.Raw,
-    'tumor_bam_id': fields.Raw,
+    'project_id': fields.Integer,
+    'normal_bam_id': fields.Integer,
+    'tumor_bam_id': fields.Integer
 }
 
 # Used because on GET /runs/<id> we want the spec and contig, too [for now].

cycledash/static/js/runs/components/forms.js

@@ -16,13 +16,12 @@ var React = require('react'),
 })(); // from ../examine/components/QueryBox.js
 
 // This instruments a form to submit via AJAX.
-// if the form successfully submit, the page will refresh.
+// if the form successfully submits, the page will refresh.
 // If there is an error, the error message is shown in an alert.
 function ajaxifyForm(form) {
   $(form).submit(function(evt) {
     evt.preventDefault();
-    var inputs = $(form).find('input').toArray();
-    inputs = inputs.concat($(form).find('textarea').toArray());
+    var inputs = $(form).find('input, textarea').toArray();
     var requestData = _.reduce(inputs, (req, input) => {
       if (input.value.length > 0) {
         req[input.name] = input.value;

cycledash/validations.py

@@ -1,3 +1,4 @@
+"""Schemas for validating API requests."""
 import json
 
 from voluptuous import (Schema, All, Any, Required, Length, Range, truth,

cycledash/views.py

@@ -3,18 +3,14 @@
 import json
 import tempfile
 
-from flask import (request, redirect, Response, render_template, jsonify,
-                   url_for, send_file)
-from sqlalchemy import select, desc, func
-import voluptuous
+from flask import request, render_template, jsonify, send_file
+from sqlalchemy import select, desc
 
 from common.relational_vcf import genotypes_to_file
 from common.helpers import tables
 
 from cycledash import app, db, api
-from cycledash.helpers import (prepare_request_data, error_response,
-                               success_response, get_secure_unique_filename,
-                               request_wants_json, camelcase_dict, from_epoch)
+from cycledash.helpers import error_response, get_secure_unique_filename, camelcase_dict
 import cycledash.genotypes
 import cycledash.comments
 import cycledash.runs

scripts/lint.sh

@@ -5,7 +5,7 @@ find cycledash/static/js tests -name '*.js' \
   | grep -v /dist/ | grep -v 'bundled' | grep -v /playground/ \
   | xargs ./node_modules/.bin/jsxhint
 
-git ls-files | grep .py \
+git ls-files | grep -e '.*.py$' \
   | xargs pylint \
   --errors-only \
   --disable=print-statement,no-member,no-name-in-module \

scripts/run-python-tests.sh

@@ -0,0 +1,13 @@
+#!/bin/bash
+# Run the Python tests
+set -o errexit
+
+DB=cycledash-test
+
+# --if-exists means to not report an error if the DB doesn't exist.
+dropdb --if-exists $DB
+
+createdb $DB
+psql $DB < schema.sql
+
+nosetests tests/python

scripts/travis-setup.sh

@@ -2,14 +2,6 @@
 # Install all deps & build everything for the Python & JS tests.
 set -o errexit
 
-DB=cycledash-test
-
-# --if-exists means to not report an error if the DB doesn't exist.
-dropdb --if-exists $DB
-
-createdb $DB
-psql $DB < schema.sql
-
 pip install -r <(grep -v 'pysam\|dpxdt\|pyensembl' requirements.txt)
 npm install
 make initenv

scripts/travis-test.sh

@@ -3,7 +3,7 @@
 set -o errexit
 
 . ./tests/ENV.sh
-nosetests tests/python
+./scripts/run-python-tests.sh
 
 ./scripts/run-js-tests.sh
 echo 'Running tests in reverse...'

tests/python/test.py

@@ -26,7 +26,6 @@ def test_camelcase():
         "camel_bi1g_cases_run1": "camelBi1gCasesRun1"
     }
     for example, expected in underscored_to_cameled_expectations.items():
-        print example, expected, camelcase(example)
         assert camelcase(example) == expected
 
 

tests/python/test_comments_api.py

@@ -4,7 +4,7 @@
 import json
 
 from cycledash import app, db
-from common.helpers import tables, from_epoch
+from common.helpers import tables, to_epoch
 
 from test_projects_api import create_project_with_name
 from test_runs_api import create_run_with_uri
@@ -86,7 +86,7 @@ def test_update_comment(self):
         r = self.app.put('/api/runs/{}/comments/{}'.format(self.run['id'], comment['id']),
                          data=json.dumps({'commentText': new_text,
                                           'authorName': new_author,
-                                          'last_modified': from_epoch(comment['last_modified'])}))
+                                          'last_modified': to_epoch(comment['last_modified'])}))
         assert r.status_code == 200
         assert json.loads(r.data)['id'] == comment['id']
         assert json.loads(r.data)['commentText'] == new_text
@@ -103,14 +103,14 @@ def test_update_with_out_of_date_timestamp(self):
         now = datetime.datetime.now()
         r = self.app.put('/api/runs/{}/comments/{}'.format(self.run['id'], comment['id']),
                          data=json.dumps({'commentText': 'blah',
-                                          'lastModified': from_epoch(now)}))
+                                          'lastModified': to_epoch(now)}))
         assert r.status_code == 409
         assert 'out of date' in json.loads(r.data)['message']
 
     def test_delete_comment(self):
         comment = create_comment_with_text(self.run['id'], 'some text')
         r = self.app.delete('/api/runs/{}/comments/{}'.format(self.run['id'], comment['id']),
-                            data=json.dumps({'lastModified': from_epoch(comment['last_modified'])}))
+                            data=json.dumps({'lastModified': to_epoch(comment['last_modified'])}))
         assert r.status_code == 200
         assert json.loads(r.data)['id'] == comment['id']
         r = self.app.get('/api/runs/{}/comments/{}'.format(self.run['id'], comment['id']))
@@ -126,6 +126,6 @@ def test_delete_with_out_of_date_timestamp(self):
         comment = create_comment_with_text(self.run['id'], 'some text')
         now = datetime.datetime.now()
         r = self.app.delete('/api/runs/{}/comments/{}'.format(self.run['id'], comment['id']),
-                         data=json.dumps({'lastModified': from_epoch(now)}))
+                         data=json.dumps({'lastModified': to_epoch(now)}))
         assert r.status_code == 409
         assert 'out of date' in json.loads(r.data)['message']

tests/python/test_projects_api.py

@@ -38,7 +38,7 @@ def test_create_duplicate_project_name(self):
         r = self.app.post('/api/projects',
                           data=json.dumps({'name': self.PROJECT_NAME}))
         assert r.status_code == 409
-        assert "duplicate key" in json.loads(r.data)['errors'][0]
+        assert 'duplicate key' in json.loads(r.data)['errors'][0]
 
     def test_create_project_without_name(self):
         r = self.app.post('/api/projects',