Installing Python 3.6 in the Docker base image throws an error at `RUN add-apt-repository ppa:jonathonf/python-3.6` since this repository has been removed. Fortunately, this Ubuntu version already has Python 3.6 installed so we can safely remove this step from the build file [as can be seen here](https://travis-ci.com/WGierke/concept-to-clinic/builds/141771352).

I ran into a few errors while trying to build the master on Travis: - Installing docker-engine errored with `The command "sudo apt-get --force-yes -qqy -o Dpkg::Options::="--force-confdef" -o Dpkg::Options::="--force-confold" install docker-engine=17.05.0~ce-0~ubuntu-trusty" failed and exited with 100` Upgrading to Ubuntu 18 on Travis solved that and allows to use a pre-installed docker-compose. - Next, the [generated base docker containers](compose/base/Dockerfile-dev) did not provide Python 3.6. Using Ubuntu 18.04 and installing Python 3.6 first solved that. - While installing `tzdata`, user interaction is needed. Adding `ENV DEBIAN_FRONTEND=noninteractive` solves that. - I added `git` to the docker containers since `npm` needs that: ``` npm ERR! Error while executing: npm ERR! undefined ls-remote -h -t ssh://git@github.com/eligrey/FileSaver.js.git npm ERR! npm ERR! undefined npm ERR! No git binary found in $PATH npm ERR! npm ERR! Failed using git. npm ERR! Please check if you have git installed and in your PATH. ``` - The refactored string interpolation in ed2c8a7bec7ce5326fdf3a5b8fbaac48facdc066 did not add leading zeroes: ```python >>> n = 5 >>> "TEST-SERIES-{:4d}".format(n) 'TEST-SERIES- 5' >>> "TEST-SERIES-{:04d}".format(n) 'TEST-SERIES-0005' ``` - There was a missing migration: ```bash +docker-compose -f local.yml run interface python manage.py makemigrations --dry-run --check Starting concept-to-clinic_postgres_1_61466b65a243 ... Starting base ... /usr/local/lib/python3.6/dist-packages/environ/environ.py:618: UserWarning: Error reading /app/.env - if you're not configuring your environment separately, check this. "environment separately, check this." % env_file) Migrations for 'images': backend/images/migrations/0006_auto_20191203_1110.py - Change Meta options on imagefile ``` - flake8 failed due to unused variables: `prediction/src/preprocess/lung_segmentation.py:118:5: F841 local variable '_' is assigned to but never used` - flake8 labeled an escaping \ for a RegEx string as invalid escape sequence. Changing the string to a RegEx string solved that Proof: [Succeeding build](https://travis-ci.com/WGierke/concept-to-clinic/builds/139559423)  ## CLA - [x] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

When forking the repository and adding it to Travis, building the master [does not succeed](https://travis-ci.com/WGierke/concept-to-clinic/builds/138996116). It fails with ``` $ sudo apt-get --force-yes -qqy -o Dpkg::Options::="--force-confdef" -o Dpkg::Options::="--force-confold" install docker-engine=17.05.0~ce-0~ubuntu-trusty E: Unable to correct problems, you have held broken packages. The command "sudo apt-get --force-yes -qqy -o Dpkg::Options::="--force-confdef" -o Dpkg::Options::="--force-confold" install docker-engine=17.05.0~ce-0~ubuntu-trusty" failed and exited with 100 during . ```

` All data are resized to 1x1x1 mm, the luminance is clipped between -1200 and 600, scaled to 0-255 and converted to uint8. A mask that include the lungs is calculated, luminance of every pixel outside the mask is set to 170. The results will be stored in 'preprocess_result_path' defined in config_training.py along with their corresponding detection labels. ` I think we already have that preprocessing steps. Converting the data to voxels, clipping the Hounsfield units that are soft tissue and rescaling the image is a very common practice among the [top solutions](https://github.com/concept-to-clinic/concept-to-clinic/tree/d447cacf57cbbe84f7b744303497030b2a92a122/docs/algorithms). Could you have a look at [lung_segmentation.py](https://github.com/concept-to-clinic/concept-to-clinic/blob/d447cacf57cbbe84f7b744303497030b2a92a122/prediction/src/preprocess/lung_segmentation.py) and [improved_lung_segmentation.py](https://github.com/concept-to-clinic/concept-to-clinic/blob/d447cacf57cbbe84f7b744303497030b2a92a122/prediction/src/preprocess/improved_lung_segmentation.py)? There already is lots of logic that might be useful for the steps you defined I think :)

Would you mind converting your code to comply with PEP8? There are [a few things](https://travis-ci.org/concept-to-clinic/concept-to-clinic/builds/333539634#L3280) that need to be fixed according to flake8 and pycodestyle :)

Are you sure this does not introduce too much bias? How do we know that nodules can also occur at the positions we're basically planting them?

This should add the possibility to benchmark a segmentation model on the LIDC dataset by calculating the average Dice coefficient, Hausdorff distance, sensitivity and specificity. I also fixed some flaws in the sensitivity and specificity calculation because we would otherwise divide by zero on certain occasions. ## Description Given a segmentation model, the method predicts each LIDC nodule segmentation mask and compares that with the ground truth mask using the aforementioned metrics. ## Reference to official issue This addresses #271. ## Motivation and Context We want to know how well the implemented models perform in order to improve them in a targeted way. ## How Has This Been Tested? I'd like to test it in the next days using a model that supports 3D convolutions. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

As demanded by #283 this adds a documentation of how to train a segmentation model using the LIDC dataset. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@caseyfitz Are you planning to merge the changes you did to the code base in [your branch](https://github.com/concept-to-clinic/concept-to-clinic/compare/investigate-prediction) at some point to the master? And by the way: nice notebook! :)

@reubano Done. Thanks!

Nice! You mentioned that you used 10-fold cross validation. Can you document somewhere how you trained and evaluated the model using your newly introduced methods?

This looks astonishing!

The current implementation takes 8.5h to run, gives an average accuracy/precision of 10% and an average loss of 3.55 :/

I'm currently adding the possibility to evaluate a classification model based on the LIDC dataset as described in #271. Furthermore, I'm currently benchmarking the model that's implemented at the moment. Even if I still haven't finished, I wanted to show you what I'm currently working at. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

While skimming through [code that still needs to be tested in the prediction service](https://wgierke.github.io/stuff/prediction/index.html), I noticed this line that prevented the file from being completely covered by tests. The reason was that the condition was never false since the `methods=['GET', 'POST']` parameter prevents any other request than a `POST` request from reaching this LOC. ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Unfortunately, I can't confirm that on - Linux Ubuntu 16.04.3 LTS - Docker version 17.09.0-ce, build afdb6d4 - docker-compose version 1.15.0, build e12f3b9

@lamby @isms Now, we just generate the standard report. If someone wants to add that the report should be uploaded to codecov.io: it worked for me by running `coverage xml` and uploading it using `bash <(curl -s https://codecov.io/bash) -t THE-CODECOV-TOKEN` to codecov. Maybe it's not necessary to specify the token when uploading from a Travis build but I'm not sure. PS: the report from the current build: prediction service: ``` Name Stmts Miss Branch BrPart Cover ------------------------------------------------------------------------------------------- __init__.py 0 0 0 0 100% config.py 22 0 0 0 100% src/__init__.py 0 0 0 0 100% src/algorithms/__init__.py 0 0 0 0 100% src/algorithms/classify/__init__.py 0 0 0 0 100% src/algorithms/classify/src/__init__.py 0 0 0 0 100% src/algorithms/classify/src/gtr123_model.py 151 1 26 1 99% src/algorithms/classify/trained_model.py 4 0 0 0 100% src/algorithms/identify/__init__.py 0 0 0 0 100% src/algorithms/identify/prediction.py 190 153 58 0 15% src/algorithms/identify/src/__init__.py 0 0 0 0 100% src/algorithms/identify/src/gtr123_model.py 284 229 68 1 16% src/algorithms/identify/trained_model.py 43 26 18 1 36% src/algorithms/segment/__init__.py 0 0 0 0 100% src/algorithms/segment/src/__init__.py 0 0 0 0 100% src/algorithms/segment/src/evaluate.py 31 0 0 0 100% src/algorithms/segment/src/models/__init__.py 0 0 0 0 100% src/algorithms/segment/src/models/segmentation_model.py 38 10 0 0 74% src/algorithms/segment/src/models/simple_3d_model.py 39 7 2 0 78% src/algorithms/segment/trained_model.py 28 0 6 0 100% src/factory.py 19 4 4 2 74% src/preprocess/__init__.py 0 0 0 0 100% src/preprocess/crop_dicom.py 21 1 8 1 93% src/preprocess/crop_patches.py 33 4 16 5 82% src/preprocess/errors.py 5 0 2 1 86% src/preprocess/extract_lungs.py 171 88 63 2 47% src/preprocess/generators.py 313 59 172 25 78% src/preprocess/improved_lung_segmentation.py 284 17 100 9 92% src/preprocess/load_ct.py 91 6 28 2 93% src/preprocess/lung_segmentation.py 139 105 36 1 20% src/preprocess/preprocess_ct.py 78 14 46 14 77% src/views.py 32 0 10 1 98% ------------------------------------------------------------------------------------------- TOTAL 2016 724 663 66 62% ``` interface service: ``` Name Stmts Miss Branch BrPart Cover --------------------------------------------------------------- backend/__init__.py 0 0 0 0 100% backend/api/__init__.py 0 0 0 0 100% backend/api/serializers.py 65 16 2 1 75% backend/api/urls.py 28 10 4 0 56% backend/api/views.py 79 7 3 0 91% backend/cases/__init__.py 0 0 0 0 100% backend/cases/enums.py 65 0 18 0 100% backend/cases/factories.py 18 0 0 0 100% backend/cases/models.py 84 0 4 0 100% backend/images/__init__.py 0 0 0 0 100% backend/images/factories.py 15 0 6 0 100% backend/images/models.py 84 1 23 0 99% config/__init__.py 0 0 0 0 100% config/settings/__init__.py 0 0 0 0 100% config/settings/base.py 33 2 0 0 94% config/settings/local.py 12 0 0 0 100% config/urls.py 10 4 4 1 50% manage.py 16 6 2 1 61% --------------------------------------------------------------- TOTAL 509 46 66 3 90% ```

[Apparently codecov.io supports Coverage.py](https://github.com/codecov/example-python). I'm currently trying to add an upload of the generated report to codecov.io on my fork. This way we can have both - a nice HTML version that can be generated locally (if need be) and a repository integration that shows us the changed coverage that would be induced by merging a branch.

@lamby I tried this "spike" for a couple of reasons. First, I don't have the rights to add such an integration like coveralls to the repo :-) Second, one would always be required to first push to GitHub to receive a test report if I understand it right. While this not only takes way longer than running the report generation locally, there are also tests that always time out on Travis which means that one can never fully rely on the report generated online. However, I understand your point.

Does something like #282 make sense? It generates a coverage report locally which could then also be persisted by Travis.

As discussed in #275, it would be helpful to generate a coverage report of the unit tests in which the total coverage and missing lines can be seen. This PR adds [Coverage.py](https://coverage.readthedocs.io/en/coverage-4.4.2/) which keeps track of which LOC haven't been tested after running the tests. It generates a `htmlcov` directory which includes the HTML files that should be served afterwards to correctly render the report. An example of such a report for the current master is [here](https://wgierke.github.io/stuff/). To persist this report generated by Travis, the project manager need to [set up Travis artifacts](https://docs.travis-ci.com/user/uploading-artifacts/). ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

I skimmed through the code and added doc strings to some functions I added whose exact method signatures and purposes I still was able to recap. More exhaustive doc string adding PRs will follow. ## Reference to official issue #274 ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Can we agree on a common [docstring format](https://stackoverflow.com/questions/3898572/what-is-the-standard-python-docstring-format)?

Would it be useful to add integrations such as [CodeCov](https://codecov.io/) to continuously monitor whether a test really adds more coverage / a feature reduces the coverage?

As discussed in #131 it would be helpful to have a consistent pipeline to evaluate prediction models. This way we get to know how well the currently implemented models are, which ones need to be improved and how well a new model performs. The pipeline should calculate the appropriate metrics that have been specified in #221 while some of the [are already available here](https://github.com/concept-to-clinic/concept-to-clinic/blob/master/prediction/src/algorithms/segment/src/evaluate.py). ## Acceptance Criteria - [ ] all evaluation methods take a model and a parameter whether to use 5-fold validation or a test set (default) - [ ] for classification calculate accuracy, Log Loss - [ ] for identification calculate accuracy, Log Loss - [ ] for segmentation calculate Dice coefficient, Hausdorff distance, Jaccard index, sensitivity, specificity - [ ] for all of them calculate data IO, disk space usage, memory usage, prediction time (if not (easily) possible, specify why and how to manually measure it) - [ ] for all of them calculate the training time Please refer to [the PR template](https://github.com/concept-to-clinic/concept-to-clinic/blob/master/.github/PULL_REQUEST_TEMPLATE.md) for further explanations of the metrics.

Hi Serhiy, can you check the slice indices, please? The z axis is the number of the slice starting at 0. This is because the image position does not always start at a fixed value. I'm taking this from the design doc. Also, the used Python library for the LIDC images interprets it like that. Thanks! On Dec 29, 2017, 10:31 PM, at 10:31 PM, Serhiy-Shekhovtsov <notifications@github.com> wrote: >I did some checking... I may be wrong, but it looks like the specified >slice is not valid. >The one we are running the test for is `{'x': 367, 'y': 349, 'z': 72}` >but the DICOM at this slice looks like a mess: > > >While when I run the test on `{'x': 367, 'y': 349, 'z': 189}` it >doesn't fail. Here is how the image looks at specified slice: > > >@WGierke, @reubano what do you think? > >-- >You are receiving this because you were mentioned. >Reply to this email directly or view it on GitHub: >https://github.com/concept-to-clinic/concept-to-clinic/issues/268#issuecomment-354502724

Thanks @pjbull ! I contacted the DrivenData team and asked for credits. If they can provide me with some, I'd like to work on this issue. Once I get their answer, I'll update this comment with the current state for transparency reasons.

@brycecf Are you still working on this one?

@isms Alright, thanks a lot for the insights and the feedback. I'll have a look at how I can solve this problem for me. Thanks!

@isms I can understand that RAM usage is not a hard constraint. However, this makes it impossible for me (and others with less than 32GB RAM) to debug (and improve as described in #130 ) the model. Do you see any possibility to address this problem?

While the nodule classification works for providing nodule locations with small values in each dimension, it fails when I tried to classify a real nodule (first one of patient LIDC-IDRI-0003). ## Expected Behavior A cancer probability should be returned. ## Current Behavior An error is thrown: ```bash (venv3.5) ➜ concept-to-clinic git:(master) ✗ docker exec -it concepttoclinic_prediction_run_259 pytest -vrsk src/tests/test_classification.py =========================================================================================== test session starts ============================================================================================ platform linux -- Python 3.6.1, pytest-3.1.3, py-1.4.34, pluggy-0.4.0 -- /usr/bin/python3.6 cachedir: .cache rootdir: /app, inifile: collected 4 items src/tests/test_classification.py::test_classify_predict_load PASSED src/tests/test_classification.py::test_classify_dicom PASSED src/tests/test_classification.py::test_classify_dicom_nodule FAILED src/tests/test_classification.py::test_classify_luna PASSED ================================================================================================= FAILURES ================================================================================================= ________________________________________________________________________________________ test_classify_dicom_nodule ________________________________________________________________________________________ dicom_path_003 = '/images/LIDC-IDRI-0003/1.3.6.1.4.1.14519.5.2.1.6279.6001.101370605276577556143013894866/1.3.6.1.4.1.14519.5.2.1.6279.6001.170706757615202213033480003264' model_path = '/app/src/algorithms/classify/assets/gtr123_model.ckpt' def test_classify_dicom_nodule(dicom_path_003, model_path): > predicted = trained_model.predict(dicom_path_003, [{'x': 367, 'y': 350, 'z': 72}], model_path) src/tests/test_classification.py:15: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ src/algorithms/classify/trained_model.py:40: in predict return gtr123_model.predict(dicom_path, centroids, model_path) src/algorithms/classify/src/gtr123_model.py:275: in predict _, pred, _ = casenet(cropped_image, coords) /usr/local/lib/python3.6/dist-packages/torch/nn/modules/module.py:224: in __call__ result = self.forward(*input, **kwargs) src/algorithms/classify/src/gtr123_model.py:215: in forward noduleFeat, nodulePred = self.NoduleNet(xlist, coordlist) /usr/local/lib/python3.6/dist-packages/torch/nn/modules/module.py:224: in __call__ result = self.forward(*input, **kwargs) src/algorithms/classify/src/gtr123_model.py:175: in forward feat = self.back2(torch.cat((rev2, out2, coord), 1)) # 64+64 /usr/local/lib/python3.6/dist-packages/torch/autograd/variable.py:897: in cat return Concat.apply(dim, *iterable) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ ctx = <torch.autograd.function.ConcatBackward object at 0x7f6c231bbb88>, dim = 1 inputs = ( (0 ,0 ,0 ,.,.) = 0.0000 0.0000 0.0000 ... 0.0000 0.0000 0.0000 0.8768 0.6747 1.5596 ... 0.5966 1.1....4682 0.4782 0.4881 0.2595 0.2695 0.2794 ... 0.4682 0.4782 0.4881 [torch.FloatTensor of size 1x3x24x24x24] ) @staticmethod def forward(ctx, dim, *inputs): ctx.dim = dim ctx.input_sizes = [i.size(dim) for i in inputs] > return torch.cat(inputs, dim) E RuntimeError: inconsistent tensor sizes at /pytorch/torch/lib/TH/generic/THTensorMath.c:2709 /usr/local/lib/python3.6/dist-packages/torch/autograd/_functions/tensor.py:317: RuntimeError ------------------------------------------------------------------------------------------- Captured stdout call ------------------------------------------------------------------------------------------- torch.Size([1, 1, 52, 96, 96]) =================================================================================== 1 failed, 3 passed in 16.99 seconds ==================================================================================== ``` See here to verify that at this position there is a nodule:  ## Steps to Reproduce [patch.txt](https://github.com/concept-to-clinic/concept-to-clinic/files/1564330/patch.txt) 1. git checkout 42ba1b121504ad318210463d422f5ca2947cbacb 2. git apply patch.txt 3. docker exec -it concepttoclinic_prediction pytest -vrsk src/tests/test_classification.py ## Possible Solution I guess it has something to do with the input size the gtr123 model expects. ## Checklist before submitting - [X] I have confirmed this using the officially supported Docker Compose setup using the `local.yml` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [X] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [X] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [X] I provided **screenshots** where appropriate - [X] I filled out all the relevant sections of this template

Good stuff! Would it be possible to provide a test for processing a LIDC scan and one for processing a LUNA scan? This would also make it easier to understand how to use the provided methods while an explanation in a pull request is harder to find than a test in the code base IMO :)

I consulted my pillow and it said that one could handle an identification as correct if the predicted nodule location is within the boundaries of the nodule, but not necessary the centroid (for LIDC images we can get the boundaries easily using [pylidc](https://github.com/pylidc/pylidc/blob/ea04c829a50e3598934e3c2f603e20969fe8381f/pylidc/Annotation.py#L227). Another way to achieve this would be to handle an identification as correct if the predicted locations are within a Ɛ-pipe of the true location (x_true - Ɛ <= x_hat <= x_true + Ɛ, y_true - Ɛ <= y_hat ...). Ɛ could vary for z since the size of this dimension differs from x and y. This brings me to my third point: the accuracy is the ratio of correct classifications from all the classifications the algorithm performed. If the algorithm only predicts one nodule out of five and the prediction is correct, its accuracy would be 100% but it still wasn't very helpful since it missed four nodules. A metric to cover this case would be the [recall](https://en.wikipedia.org/wiki/Precision_and_recall) which can also easily be computed. Any thoughts on that? :)

Maybe we should proceed in small steps by e.g. first adding evaluation methods that cover most of the metrics introduced in #221 so we can have a more standardized way to determine those metrics? This way we would also be able to quickly determine the quality of the current implementations of the identification, classification and segmentation algorithms. This would in turn make it easier to focus first on the algorithm that is performing the worst so far. Any thoughts @isms @reubano @lamby ?

@reubano one thing that just came to my mind: isn't identification about predicting a (numerical) location of a nodule? How should we then calculate the accuracy based on that? From my understanding the accuracy can only be used for classification problems, since here we have a regression problem something like the mean squared error would be more appropriate, wouldn't it? Or do you want to put every x, y and z value in their own classes? This way the accuracy wouldn't differ for a prediction in which e.g. x_hat is x_true+1 and a prediction where x_hat is x_true+100.

@lamby can this issue be closed then?

I think so, yes

There was no method implemented how to get the origin of a DICOM image series which resulted in an error. I initially set it to [0, 0, 0] but due to my lack of domain knowledge I don't know how to correctly calculate it from DICOM images. If you know the answer, feel free to comment :) ## Reference to official issue This references #229 - classification doesn't work on DICOM images. ## How Has This Been Tested? I added the test given in #229 ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Apparently the segmentation issues noted in #229 have been fixed as a byproduct of #237 . This PR just adds the test given in #229 to show that all tests pass. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@reubano could you do me a favor and check out my branch [229_fix_segmentation](https://github.com/concept-to-clinic/concept-to-clinic/compare/master...WGierke:229_fix_segmentation?expand=1)? All I did was to apply the patch for the segmentation tests and merge the current master. The outcome was that all segmentation tests passed (I also ran slow tests): ```bash ➜ concept-to-clinic git:(229_fix_segmentation) ✗ docker-compose -f local.yml run prediction pytest -vrsk src/tests/test_segmentation.py Starting base ... Starting base ... done ======================================== test session starts ======================================== platform linux -- Python 3.6.1, pytest-3.1.3, py-1.4.34, pluggy-0.4.0 -- /usr/bin/python3.6 cachedir: .cache rootdir: /app, inifile: collected 7 items src/tests/test_segmentation.py::test_correct_paths PASSED src/tests/test_segmentation.py::test_segment_predict_load PASSED src/tests/test_segmentation.py::test_segment_dicom PASSED src/tests/test_segmentation.py::test_segment_luna PASSED src/tests/test_segmentation.py::test_nodule_segmentation PASSED src/tests/test_segmentation.py::test_lung_segmentation PASSED src/tests/test_segmentation.py::test_stop_timeout PASSED ==================================== 7 passed in 514.55 seconds ===================================== ``` I think the issues behind the failing segmentation tests at the moment of opening this issue could have been resolved in #237 . Can you confirm?

I really appreciate your offer to run the identification tests on your machine and to send me possible occurring stack traces @lamby ! :grin:

@lamby Done. I changed test_docker for faster testing purposes, but that has been reverted again :) Shouldn't have commited that.

I fixed the identification of LUNA nodules as far as it gets on my machine. This means that currently all tests in `src/tests/test_identification.py` are failing for me since I don't have 21 GB of RAM. Maybe someone else can execute them and send me tracebacks for further error investigation? ## Description The problem was that sometimes the direct path to a MHD file was handed to the function instead of the path to the directory. The made changes should support now both alternatives. ## Reference to official issue This addresses #229 - identification test(s) should pass for luna scans. ## How Has This Been Tested? I added the test given in #229 . ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

This PR fixed the case that a `Termination` exception can be thrown in tests that should timeout. ## Reference to official issue This addresses #247 . ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

As discussed in #239 , apparently #243 introduced a race condition that involves tests that should timeout. Sometimes, a `TimeoutExit` is thrown (which is correctly caught) and sometimes a `Termination` exception is thrown which is currently now caught and makes the tests fail (e.g. [here](https://travis-ci.org/concept-to-clinic/concept-to-clinic/builds/306144770?utm_source=github_status&utm_medium=notification)) ## Expected Behavior A test that creates a timeout should be caught by `xfail` and thus not fail. ## Possible Solution Only throw `TimeoutExit` exceptions since there is no reason to throw two kinds of exceptions when a test should be killed. ## Checklist before submitting - [X] I have confirmed this using the officially supported Docker Compose setup using the `local.yml` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [X] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [X] I filled out all the relevant sections of this template

I'm not able to do that but you can do a `git commit --amend` and `git push -f` to manually restart the Travis test. However, give me a second to file an issue for that and (hopefully) to directly propose a patch :)

@reubano do you think this can be closed as well?

Was that a LGTM? :grin:

From [Allow compose to run arbitrary commands at the host level after or before containers are started](https://github.com/docker/compose/issues/2012): `command is always run in a container. docker-compose is not a tool for running commands on the host.` Seems like we need to put your suggested commands into an extra bash script :/

So has this now been implemented in 11d04b0?

Nvm, I think I got it :)

We don't want to completely skip slow tests since this can hide bugs (e.g. #191). Instead, we want to start the slow tests but skip them after a specified period of time. ## Description If the environment variable RUN_SLOW_TESTS is set to True, all tests should be run. Otherwise tests that exceed the time specified by the variable TESTS_TIMEOUT (default is 15 seconds) are skipped without causing an error. ## Reference to official issue This addresses #195 ## Motivation and Context It was necessary to create a new [hook](https://github.com/concept-to-clinic/concept-to-clinic/compare/master...WGierke:195_stop_slow_tests?expand=1#diff-507495becfd1e874bad6e0b1cf4437b2R115) that checks whether a test should timeout, that kills the test when exceeding the timeout and then returns that the test is not failing (if the timeout is the only reason for the test exit). It wasn't possible to just use pytest-timeout and the xfail marker since pytest-timeout does not raise an exception when killing a test which could be caught by the xfail marker. ## How Has This Been Tested? I added a [test](https://github.com/concept-to-clinic/concept-to-clinic/compare/master...WGierke:195_stop_slow_tests?expand=1#diff-ed63136572f32bc5f7500b864f9bdfa9R60) that should always timeout (if we want slow tests to timeout), otherwise it raises an uncaught exception. The tests that are failing are expected to fail (as stated in the issue and #229 ) namely `src/tests/test_endpoints.py::test_identify`, `src/tests/test_identification.py::test_identify_nodules_001` and `src/tests/test_identification.py::test_identify_nodules_003`. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Cool stuff! Is it possible to introduce tests for the front-end as well?

It would take 1 GB to connect all the input neurons to just 1 output neuron. Using max pooling and convolutional layers would of course crunch the number of trained weights. However, even the [2nd place of the NDSB 2017](http://juliandewit.github.io/kaggle-ndsb2017/) only used an input shape of 64x64x64 mm³ at max (which needed a Tesla K80 with 12 GB to train).

If I understand [this](https://stackoverflow.com/questions/46766899/pytest-timeout-fail-test-instead-killing-whole-test-run) correctly, it doesn't appear to be so easy to catch the case when pytest-timeout stops a method because the SIGALRM signal does not produce an exception. I was able to implement the remaining logic but currently I'm struggling with somehow catching that timeout exception: `git checkout bcb743766386794bf373bd14972cb3b05c60d727` `git apply patch.txt` [patch.txt](https://github.com/concept-to-clinic/concept-to-clinic/files/1485956/patch.txt) Does someone have an idea how to still let tests pass even when they timeout?

I took a quick glance at the identification problems. When running the tests, I get a `RuntimeError: $ Torch: not enough memory: you tried to allocate 21GB. Buy new RAM! at /pytorch/torch/lib/TH/THGeneral.c:270`. This basically means that our client / web server (depending on where the project will be deployed in the end) needs at least 21 GB of RAM. Is it realistic to assume that this will be the case? Otherwise we might need to crunch the model a bit ... PS: :+1: for providing tests that need to pass :)

@lamby Yes ;) It should be done now.

Once #237 is merged, I'd like to work on "segmentation test(s) should pass for luna scans".

The problem was that we haven't had a standardized way to handle training data. Now, the scans and binary nodule images have the shape 512x512x1024 so all DICOM images that have been rescaled to voxels fit into that. However, since this is a giant input matrix which can easily blow up the memory when training a neural network I added a `SegmentationModel` wrapper that takes this shape as an input and also predicts this shape, but the models that implement this interface can also rescale / crop / ... the input data as long as the predicted output is again of size 512x512x1024. For example consider the [`Simple3DModel`](https://github.com/concept-to-clinic/concept-to-clinic/compare/master...WGierke:187_fix_segmentation?expand=1#diff-7ebde87c389dab64f0c9e8a81e6b6391R49): it scales the input by 1/4 for each axis, learns a model based on that and after predicting it scales the predicted binary nodule mask again by factor 4 such that the output shape equals the given training data shape again. Note that I mainly moved the code from `models.py` to the classes in the `models/` directory to accomplish this. ## Reference to official issue This references #187 and #151 since I wasn't able to clearly separate them. ## How Has This Been Tested? I added the piece of code that threw an error given in #187 as a test. All tests pass. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

I have some pretty obvious questions: 1. If you could wish for a piece of software that could help you detecting lung cancer nodules, what would be its most useful features and how would it be different from any possible "competitors" that already exist? 2. What would be the most important properties such a software has to fulfill such that you would use it on a regular basis (intuitive user interface, responsive, high accuracy, ...?)

`The identification tests should fail on travis even when RUN_SLOW_TESTS isn't set to true.` Assuming that an error occurs in the seconds before the timeout, right? Otherwise the test passes because the timeout is not seen as an error?

` this is about waiting for things to spin-up, no` Exactly, that's what I meant. Sorry for the confusion :)

Okay but what if a port won't be opened because the service died for some reason? In that case we test endlessly which might be no problem on Travis since there is a timeout but not on local computers.

This assigns a static name to the generated `base` image such that docker still works if your directory is named something else than "concept-to-clinic". This addresses #222 . ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Setting the image name automatically would be the cleanest solution IMO. [SO](https://stackoverflow.com/questions/32230577/how-do-i-define-the-name-of-image-built-with-docker-compose) says `docker-compose --project-name foo build bar` would build the image foo_bar no matter what the parent directory is named. [This answer](https://stackoverflow.com/questions/32230577/how-do-i-define-the-name-of-image-built-with-docker-compose/33951705#33951705) even shows how to do that using the docker-compose YAML file.

Good to hear that it works now :)

Can you give us the output of `docker images | grep concepttoclinic_base` ? I think the base image might get another name at your computer (since it works on Travis).

:D Okay so how about, instead of `sleep 60`, we try to connect to the 4 ports and after 60 seconds we time out and exit with 1, otherwise we perform the tests?

@lamby Thanks for the hint, that indeed is a way better solution to test for unsuccessful responses. However, apparently one has to do some more bash magic than expected. The docker containers must be started so the ports are served at localhost. However, the `compile_docs` service is producing such a big output that it hits the Travis log limit of 4MB and the test fails (apart from that, fixing `compile_docs` might be a new separate issue?). Thus, I'm piping the output to /dev/null. While the tests now work on my machine, they're still failing on Travis: ```bash ➜ concept-to-clinic git:(160_route_errors) ✗ git log | head -n 1 commit 0e8525940f3fda572745e400f8cb1fe01b35c329 ➜ concept-to-clinic git:(160_route_errors) ✗ bash tests/test_docker.sh ... + docker-compose -f local.yml run interface python manage.py makemigrations --dry-run --check Starting concepttoclinic_postgres_1 ... Starting base ... Starting base ... done Starting concepttoclinic_prediction_1 ... done Postgres is up - continuing... /usr/local/lib/python3.6/dist-packages/environ/environ.py:618: UserWarning: Error reading /app/.env - if you're not configuring your environment separately, check this. "environment separately, check this." % env_file) No changes detected + sleep 60 + docker-compose -f local.yml up + python tests/test_routes.py Fetching http://localhost:8000/ ... Fetching http://localhost:8000/api ... Fetching http://localhost:8080/ ... Fetching http://localhost:8001/ ... Fetching http://localhost:8001/classify/predict/ ... Fetching http://localhost:8001/identify/predict/ ... Fetching http://localhost:8001/segment/predict/ ... Fetching http://localhost:8002/ ... ➜ concept-to-clinic git:(160_route_errors) ✗ echo "$?" 0 ``` I'd be happy over any solution suggestions :)

I added a simple Python script that can test whether routes error. It is highly customizable (e.g. one could add to check for the occurrence of the word "error" or "exception" etc.). ## Reference to official issue This addresses #160 ## Motivation and Context Currently, the Travis test passes even if some routes are non-functional (like http://0.0.0.0:8001/). ## How Has This Been Tested? It's currently not passing due to the unfixed issue #203 . ```bash ➜ concept-to-clinic git:(160_route_errors) ✗ bash tests/test_docker.sh ... + python tests/test_routes.py http://localhost:8001/ returns 500 + echo 'ERROR: test_routes did not pass. Check above for details.' ERROR: test_routes did not pass. Check above for details. + exit 1 ``` ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Nice. Thanks and sorry :)

Would it make sense to write a test that checks whether the traceback still exists when visiting the page? This would enable us to notice when the page breaks again.

I'm having at look at this.

I guess the `docs` service should still use the cache that was built by the `interface` service (instead of re-downloading all the Python dependencies)? Do you have any ideas how to accomplish that?

First, a base image is built that installs all needed pip requirements. The docker containers, that need parts of the requirements (prediction, interface and documentation services), then are built on top of that base image. Thus, building the documentation service doesn't require to install all pip requirements again resulting in a great speed-up of the container building. ## Reference to official issue This addresses #159 ## How Has This Been Tested? `docker-compose -f local.yml build && docker-compose -f local.yml up` builds all containers properly and fires them up. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

( :+1: for the first-pr-only label)

@reubano Done. Travis had some temporary issues so I had to `git commit --amend && git push -f` several times to restart the builds. In case you received any emails about me pushing the same commit message again and again to the branch ;)

Thanks @reubano - works like a charm :)

A friend of mine proposes that we should build our own base image which is based on the union of the pip requirements. Those, the base image is only supposed to be compiled once. If services use different versions of a library, they have to install them themselves after the base image was created. Just throwing in my two cents :)

Thanks a lot @reubano . Much appreciated :)

@lamby @reubano Would you mind reviewing again, please?

We use the scans of the [LIDC](https://wiki.cancerimagingarchive.net/display/Public/LIDC-IDRI) dataset. You can already find 3 patient scans in [tests/assets/test_image_data/full](https://github.com/concept-to-clinic/concept-to-clinic/tree/master/tests/assets/test_image_data/full). The neat thing is that there is a library (pylidc) that makes it really easy to query scan and annotation information. After downloading the dataset, set up pylidc by creating a `.pylidc` file locally that points to your local LIDC directory (like [here](https://github.com/concept-to-clinic/concept-to-clinic/pull/147/files#diff-ff90b371f444f3305e167198719a5333)) and run [prepare_training_data](https://github.com/concept-to-clinic/concept-to-clinic/pull/147/files#diff-24a9ce10839958291d5e9180703e0d79R9) which will save binary lung masks of each patient in which 1 indicates that the voxel was annotated to be cancerous by the radiologist(s). For further questions please refer to the [gitter](https://gitter.im/concept-to-clinic/) chat, so succeeding contributors can also find that information and don't have to look through closed pull requests ;) ( @lamby The aforementioned workflow might fit well into the docs as soon as the PR is merged, don't you think so?)

Hi @rracinskij , Thanks for your question! I just expected a 3D model to perform well as it also can capture the context between the scan slices in contract to a vanilla 2D model. But I think that any model that yields good results is a valuable contribution (especially since we currently don't have any nicely working segmentation model so far) :)

Is there anything left that prevents this issue from getting closed?

I added @isms' post to the documentation. It references issue #153 .  ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

I fixed the RuntimeError described in #152 by setting the appropriate variables. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@lamby Would you mind reviewing again?

`docker-compose build` throws a RuntimeError in the prediction service: ```bash prediction_1 | Traceback (most recent call last): prediction_1 | File "/usr/local/bin/flask", line 11, in <module> prediction_1 | sys.exit(main()) prediction_1 | File "/usr/local/lib/python3.6/dist-packages/flask/cli.py", line 513, in main prediction_1 | cli.main(args=args, prog_name=name) prediction_1 | File "/usr/local/lib/python3.6/dist-packages/flask/cli.py", line 380, in main prediction_1 | return AppGroup.main(self, *args, **kwargs) prediction_1 | File "/usr/local/lib/python3.6/dist-packages/click/core.py", line 676, in main prediction_1 | _verify_python3_env() prediction_1 | File "/usr/local/lib/python3.6/dist-packages/click/_unicodefun.py", line 118, in _verify_python3_env prediction_1 | 'for mitigation steps.' + extra) prediction_1 | RuntimeError: Click will abort further execution because Python 3 was configured to use ASCII as encoding for the environment. Consult http://click.pocoo.org/python3/for mitigation steps. prediction_1 | prediction_1 | This system supports the C.UTF-8 locale which is recommended. prediction_1 | You might be able to resolve your issue by exporting the prediction_1 | following environment variables: prediction_1 | prediction_1 | export LC_ALL=C.UTF-8 prediction_1 | export LANG=C.UTF-8 prediction_1 exited with code 1 ``` ## Possible Solution Adding ```bash LC_ALL=C.UTF-8 LANG=C.UTF-8 ``` as suggested to local.yml seems to fix the problem. ## Steps to Reproduce `git fetch origin` `git checkout master` `git pull` `docker-compose -f local.yml build` `docker-compose -f local.yml up` Can someone reproduce this? ## Checklist before submitting - [X] I have confirmed this using the officially supported Docker Compose setup using the `local.yml` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [X] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [X] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [X] I provided **screenshots** where appropriate - [X] I filled out all the relevant sections of this template

We want to automatically segment nodules as described #3 . To train a machine learning algorithm, the model expects a fixed input and output size. This is challenging because on the one hand, the training data, which is represented by CT scans scaled to mm voxels, varies greatly between the different scans so small scaled scans have to be padded appropriately. On the other hand, having training data with shape 512x512x512 can blow up the memory consumption of the model if we're e.g. trying to implement a [3D U-Net](https://github.com/concept-to-clinic/concept-to-clinic/pull/147/files#diff-a5d50c079ff0f39a37a401c2499748a0R35). Currently, a fixed input size is implemented [here](https://github.com/concept-to-clinic/concept-to-clinic/pull/147/files#diff-6dba1c59850535ca155232ad51813988R23). ## Expected Behavior Find an appropriate shape for training data such that all the LIDC scans can be imported after being rescaled to mm voxels. Show that you can still train a classifier on top of that which is not too demanding regarding GPU memory while training time (preferably even using a convolutional neural network since algorithms based on them are state-of-the-art solutions for pattern detection in many areas). ## Possible Implementation You could try to use [get_max_scaled_dimensions](https://github.com/concept-to-clinic/concept-to-clinic/pull/147/files#diff-6dba1c59850535ca155232ad51813988R32) which iterates over all rescaled LIDC images and returns the maximal dimensions of them. Furthermore, you could try to use [cropping layers](https://faroit.github.io/keras-docs/1.2.2/layers/convolutional/#cropping3d) provided by keras.

@lamby I already updated the `get_dicom_paths` method [here](https://github.com/concept-to-clinic/concept-to-clinic/pull/147/files#diff-24a9ce10839958291d5e9180703e0d79R17) (didn't mention that in the commit message, sorry). Why does removing the ```python if CROP_SIZE != CUBE_SIZE: cube_img = helpers.rescale_patient_images2 ``` stuff need comments? If we're honest, this shouldn't even have made it to the current code base - I introduced it in #118 but only noticed now that it currently isn't executed and was probably used by Julian de Wit to play around with the `CROP_SIZE`.

Thanks @lamby and @reubano . Your proposed changes are way cleaner :)

Thanks @dchansen ! Do you mind sharing some stats about what's the input shape of your V-NET and how much GPU memory does it consume? Because I think that the model took more than 8 GB of GPU memory for an input shape of 512x512x512. As described in #3 I think using cross validation on the LIDC dataset would make sense. The evaluation metrics introduced by #146 could be used to give a broader overview of how well the different approaches perform. Maybe some radiologists prefer precision (How many selected nodules are relevant?) over recall (How many relevant nodules are selected?) because they don't want to waste time with false positives, or vice versa because they have the time to evaluate false positives and prefer to have as many potential nodules in the suggestion as possible.

As demanded by the ticket, I implemented a very simple segmentation algorithm that can be trained using Keras, its model is saved in the `assets` folder and that can predict nodules. The model is still pretty shitty, so hopefully there are some more experiences data scientists than me around to improve it :) However, the "environment" to train the model should be implemented by this PR. ## Description I created training data pairs based on the LIDC dataset using pylidc. The label image is a binary mask whose pixels are 1 if there is an annotation in the LIDC dataset whose malice is bigger or equal to 3 (out of 5). The label images are saved in the `prediction/src/algorithms/segment/assets/` directory. Since some 3D arrays are hard to imagine, I added two methods that help to visualize these arrays by navigating through one dimension and simultaneously displaying the remaining two ones:  `display_training_pair()` shows the input image and the binary output mask. Note the segmented annotation on the right hand side and the slider for the Z axis at the bottom. When the mask is being applied on the input data, `cuboid_show_slider` shows us:  [prepare_training_data() in prediction/src/algorithms/segment/src/data_generation.py](https://github.com/concept-to-clinic/concept-to-clinic/compare/master...WGierke:3_segmentation_algorithm_lung?expand=1#diff-24a9ce10839958291d5e9180703e0d79) saves a npy file including the binary segmented mask of a lung for each patient. [train() in prediction/src/algorithms/segment/src/training.py](https://github.com/concept-to-clinic/concept-to-clinic/compare/master...WGierke:3_segmentation_algorithm_lung?expand=1#diff-6dba1c59850535ca155232ad51813988) trains the model based on the previously generated binary masks and the rescaled volume images of each DICOM scan. The model configuration is saved in [prediction/src/algorithms/segment/src/model.py](https://github.com/concept-to-clinic/concept-to-clinic/compare/master...WGierke:3_segmentation_algorithm_lung?expand=1#diff-a5d50c079ff0f39a37a401c2499748a0). ## Reference to official issue This addresses #3 ## Motivation and Context We want to suggest the radiologist to have a look at nodules that are automatically found by an algorithm of us. ## How Has This Been Tested? I wrote a [test](https://github.com/concept-to-clinic/concept-to-clinic/compare/master...WGierke:3_segmentation_algorithm_lung?expand=1#diff-ed63136572f32bc5f7500b864f9bdfa9) that checks whether the response of the segmentation endpoint has the correct format. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well I'd highly appreciate any feedback!

@lamby same here - is something preventing this issue from getting closed? :)

@lamby Is there still anything missing to fulfill the issue? Otherwise it could be closed I guess? :)

As discussed in #142, to facilitate evaluating predicted segmentation images, it would make sense to implement metrics that compare the predicted images with the ground truth in order to compare approaches more efficiently. Some measures/distances that should at least be considered for this are sensitivity, specificity, Dice coefficient and Hausdorff distance. [The Multimodal Brain Tumor Image Segmentation Benchmark (BRATS)](https://www.ncbi.nlm.nih.gov/pmc/articles/PMC4833122/) chapter "E. Evaluation Metrics and Ranking" describes them well. ## Expected Behavior Find metrics and distances that measure the deviation of a predicted, segmented image from its ground truth. Implement them so 2D as well as 3D images / data cubes can be passed to them. ## Possible Solution [scipy.spatial.distance](https://docs.scipy.org/doc/scipy/reference/spatial.distance.html#module-scipy.spatial.distance) seems to already offer some nice stuff. ## Acceptance criteria - [ ] explain why you chose to implement the selected metrics / distances - [ ] implement the comparison methods for 2D and 3D numpy arrays - [ ] write appropriate tests and show that the methods work

Yep, we're definitely supposed to refine the lung segmentation. Have a look at e.g. patient 0001 and the nodule at 315.04852321, 365.87447257, -116.12078059 (slice ~91):  It's state is labeled as solid, which isn't included in the current lung segmentation:  However, some of the slices of the nodule are included in the lung segmentation but currently this would still falsify statistics such as the predicted volume of a nodule as to be implemented in #3 .

Nice stuff! Do you have some measurement for the accuracy of your segmentation algorithm on the LIDC dataset? That would make it easier to compare it against further approaches :)

Instead of rebasing, I prefer to squash the commits like [so](https://gist.github.com/patik/b8a9dc5cd356f9f6f980). It fulfills the purpose as well, namely having only 1 commit in the end :)

@isms I added a [test](https://github.com/concept-to-clinic/concept-to-clinic/pull/133/files#diff-e6c519f70a6bee9cab764b751df70348) that checks whether the annotations of the local LIDC images are inside the segmented lung masks. I'm using [pylidc](https://github.com/pylidc/pylidc) for that which makes it easier to query attributes of the LIDC images (like annotations). However, the library has to be installed after the other requirements, otherwise it throws some `ModuleNotFoundError`s.

@isms Oh alright, sorry, now I got it. Yes, thank definitely makes sense. I'll add that :)

I added an initial documentation of the Owkin algorithm. The algorithm scored 10th place during the DSB 2017. Unfortunately, there is no technical report or blog post about the approach so far. Thus, I had to derive the algorithm design from the source code and black-box models only, which resulted in a shorter documentation than for other algorithms. ## Reference to official issue This pull request addresses #28 ## Motivation and Context At some point we need to implement an algorithm that can segment the nodules and detect whether they are malicious or not. It makes sense to first evaluate the top 10 solutions for the Data Science Bowl 2017 regarding how performant they are and how easy it would be to use them in our application. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@isms Thanks for your input. If I understand you correctly, the issue with the current identification algorithm by De Wit will be that it only considers tissue that is inside this mask, so it's not possible at all to find nodules outside the lung. I could rewrite the identification algorithm to consider the whole image and not just the lung but what would we expect to happen in that case? Only detecting nodules in an area that corresponds to the lung would be nice. However, if the algorithm detects nodules in areas that do not belong to the lung, the reason could also be that the model simply wasn't trained to distinguish between lung and not lung on its own - so the model could also be "false".

@lamby I filed #134 . You're right, it blocked this PR when the front-end of the lung orientation selection was still part of this PR. Since that moved to #125 (and the issue has been filed), I'd say that this PR shouldn't be blocked anymore?

This issue is a follow-up of the discussion from [here](https://github.com/concept-to-clinic/concept-to-clinic/pull/101#issuecomment-330208795). [Cross-Site Request Forgery (CSRF) is an attack that forces an end user to execute unwanted actions on a web application in which they're currently authenticated.](https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)) As a defensive measure, one configures the server to send a random token to the client which has to be sent back when e.g. a POST request is performed. This ensures that the origin of the POST request really is the own application. To solve this issue, please send a CSRF token by default to the (Vue.js) client and pass it back when performing POST/PUT/... requests to the server, by default as well. ## Expected Behavior The server should set a CSRF token per default with every response. When sending data to the server (e.g. to select the lung orientation as specified in #34), this token should be added automatically to the payload. ## Current Behavior We're not using CSRF tokens. ## Possible Implementation Letting Django respond to every request with a CSRF token seems pretty easy by e.g. adding `'config.middleware.ForceCsrfCookieMiddleware',` to `MIDDLEWARE` and adding ``` from django.utils.deprecation import MiddlewareMixin from django.middleware.csrf import get_token class ForceCsrfCookieMiddleware(MiddlewareMixin): """ Forces the CSRF cookie to be set for each request """ def process_request(self, request): get_token(request) ``` in `interface/config/middleware.py`. [Reference](https://stackoverflow.com/questions/32231607/django-forcing-csrf-token-on-all-responses) Regarding #125 , letting Vue (with axios) respond with the correct CSRF token seems to be as easy as ``` import axios from 'axios'; axios.defaults.xsrfCookieName = 'csrftoken' axios.defaults.xsrfHeaderName = 'X-CSRFToken' ``` [Reference](https://cbuelter.wordpress.com/2017/04/10/django-csrf-with-axios/) ## Acceptance criteria - [ ] the token is added to every Django response by default - [ ] the token is added to every payload send to the server by default - [ ] appropriate tests

@reubano To me it looks like PR #132 is not directly connected to this issue as the PR is more about implementing ways to distort input data to artificially generate more data to train on. The sentence that references this issue (`The aforementioned pipeline mainly consists of Hounsfield scaling, lungs segmentation (#120), CT re-orientation and a batch of trivial, yet resource consumptive spacial operations: zoom, rotation, shear, shift, flip and combinations of them. The convenient solution for the latter process is to build a generator which will yield a processed patches via affine transformations.`) sounds to me like the author simply wanted to describe the context in which the data generator fits. Telling from the code, I don't see a possibility to segment lungs - neither in `src/preprocess/lungs_segmentation.py` (which doesn't exist in the PR) nor anywhere else - or am I overseeing something @vessemer ?

@reubano Why has this been closed? Should I discard PR #133 now?

I refactored the current identification algorithm that's based on the approach of [Julian de Wit](https://github.com/concept-to-clinic/concept-to-clinic/blob/master/docs/algorithm_de_wit.md) which uses Hounsfield values between -1000 and 400 to segment lungs. As a byproduct, the algorithm already saves images of pre-processed scans and the related lung masks. I refactored the structure so the segmentation method can be accessed easier by other algorithms as well. ## Reference to official issue This addresses #120 . ## Motivation and Context Every nodule identification algorithm is supposed to somehow segment the lung first. Thus, it makes sense to have one separate function that is shared among the different identification algorithms. ## How Has This Been Tested? I ran the [nodule identification test](https://github.com/concept-to-clinic/concept-to-clinic/blob/master/prediction/src/tests/test_endpoints.py#L76) which saved files of the segmented lungs as PNG files to `data/extracted/USER_ID/`. I constructed the following GIF based on them, which shows how well the lung got segmented.  I'm very open for suggestions for more "code-heavy" tests ;) ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Letting Django respond to every request with a CSRF token seems pretty easy by e.g. adding `'config.middleware.ForceCsrfCookieMiddleware',` to `MIDDLEWARE` and adding ``` from django.utils.deprecation import MiddlewareMixin from django.middleware.csrf import get_token class ForceCsrfCookieMiddleware(MiddlewareMixin): """ Forces the CSRF cookie to be set for each request """ def process_request(self, request): get_token(request) ``` in `interface/config/middleware.py`. Regarding #125 , letting Vue (with axios) respond with the correct CSRF token seems to be as easy as ``` import axios from 'axios'; axios.defaults.xsrfCookieName = 'csrftoken' axios.defaults.xsrfHeaderName = 'X-CSRFToken' ``` However, is CSRF really something that should block this PR about handling the lung orientation in the back-end? Shouldn't the CSRF token management be added in another PR?

No, thanks.

Re-cloning the repository "solved" it. What the heck. But thanks for your input @vessemer !

Great stuff! If you add one more empty line to `serializers.py:38`, the Travis tests should pass :) ``` (venv3.5) ➜ concept-to-clinic_old git:(71855eb) flake8 interface && flake8 prediction interface/backend/api/serializers.py:38:1: E302 expected 2 blank lines, found 1 ```

Thanks for trying to reproduce it @vessemer . The tests work at another computer of mine, but my main tower is still having troubles. I `chown`ed everything to my user, restarted `dockerd` and deleted all images and containers. No success so far :/ I'm currently cloning the repository to a new location so I can test it there again. If the tests still fail, I'll re-install docker.

It displays a summary of skipped tests :) otherwise, every attempt to print something by a test is captured and only showed if the test fails.

I replaced `local.py` with `local.yml` in the issue template. This addresses #127 . ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

When running the tests locally, I'm getting 4 failed tests due to a RuntimeError in SimpleITK (on the current master). ## Expected Behavior The tests should pass. ## Current Behavior 4 tests fail due to SimpleITK: ``` (venv3.5) ➜ concept-to-clinic git:(master) ✗ sudo bash tests/test_docker.sh + docker-compose -f local.yml run prediction pytest ======================================== test session starts ======================================== platform linux -- Python 3.6.2, pytest-3.1.3, py-1.4.34, pluggy-0.4.0 rootdir: /app, inifile: collected 26 items src/tests/test_calculate_volume.py ... src/tests/test_classify_trained_model_predict.py .. src/tests/test_crop_dicom.py . src/tests/test_endpoints.py ........ src/tests/test_load_ct.py FFFF src/tests/test_load_dicom.py .... src/tests/test_preprocess_dicom.py .... ============================================= FAILURES ============================================== ________________________________________ test_load_metaimage ________________________________________ metaimage_path = '../images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797' dicom_path = '../images/LIDC-IDRI-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.298806137288633453246975630178/1.3.6.1.4.1.14519.5.2.1.6279.6001.179049373636438705059720603192' def test_load_metaimage(metaimage_path, dicom_path): path = glob(os.path.join(metaimage_path, '*.mhd'))[0] > ct_array, meta = load_ct.load_metaimage(path) src/tests/test_load_ct.py:25: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ src/preprocess/load_ct.py:72: in load_metaimage meta = SimpleITK.ReadImage(path) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ args = ('../images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797.mhd',) def ReadImage(*args): """ ReadImage(VectorString fileNames, itk::simple::PixelIDValueEnum outputPixelType) -> Image ReadImage(std::string const & filename, itk::simple::PixelIDValueEnum outputPixelType) -> Image ReadImage is a procedural interface to the ImageSeriesReader class which is convenient for most image reading tasks. Note that when reading a series of images that have meta-data associated with them (e.g. a DICOM series) the resulting image will have an empty meta-data dictionary. It is possible to programmatically add a meta-data dictionary to the compounded image by reading in one or more images from the series using the ImageFileReader class, analyzing the meta-dictionary associated with each of those images and creating one that is relevant for the compounded image. See: itk::simple::ImageFileReader for reading a single file """ > return _SimpleITK.ReadImage(*args) E RuntimeError: Exception thrown in SimpleITK ReadImage: /tmp/SimpleITK/Code/IO/src/sitkImageReaderBase.cxx:82: E sitk::ERROR: Unable to determine ImageIO reader for "../images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797.mhd" /usr/local/lib/python3.6/site-packages/SimpleITK/SimpleITK.py:8332: RuntimeError ___________________________________________ test_load_ct ____________________________________________ metaimage_path = '../images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797' dicom_path = '../images/LIDC-IDRI-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.298806137288633453246975630178/1.3.6.1.4.1.14519.5.2.1.6279.6001.179049373636438705059720603192' def test_load_ct(metaimage_path, dicom_path): ct_array, meta = load_ct.load_ct(dicom_path) assert isinstance(ct_array, np.ndarray) assert ct_array.shape[2] == len(meta) > ct_array, meta = load_ct.load_ct(metaimage_path) src/tests/test_load_ct.py:47: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ src/preprocess/load_ct.py:103: in load_ct meta = load_metaimage(mhd_pattern, voxel=voxel) src/preprocess/load_ct.py:72: in load_metaimage meta = SimpleITK.ReadImage(path) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ args = ('../images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797.mhd',) def ReadImage(*args): """ ReadImage(VectorString fileNames, itk::simple::PixelIDValueEnum outputPixelType) -> Image ReadImage(std::string const & filename, itk::simple::PixelIDValueEnum outputPixelType) -> Image ReadImage is a procedural interface to the ImageSeriesReader class which is convenient for most image reading tasks. Note that when reading a series of images that have meta-data associated with them (e.g. a DICOM series) the resulting image will have an empty meta-data dictionary. It is possible to programmatically add a meta-data dictionary to the compounded image by reading in one or more images from the series using the ImageFileReader class, analyzing the meta-dictionary associated with each of those images and creating one that is relevant for the compounded image. See: itk::simple::ImageFileReader for reading a single file """ > return _SimpleITK.ReadImage(*args) E RuntimeError: Exception thrown in SimpleITK ReadImage: /tmp/SimpleITK/Code/IO/src/sitkImageReaderBase.cxx:82: E sitk::ERROR: Unable to determine ImageIO reader for "../images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797.mhd" /usr/local/lib/python3.6/site-packages/SimpleITK/SimpleITK.py:8332: RuntimeError __________________________________________ test_load_meta ___________________________________________ metaimage_path = '../images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797' dicom_path = '../images/LIDC-IDRI-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.298806137288633453246975630178/1.3.6.1.4.1.14519.5.2.1.6279.6001.179049373636438705059720603192' def test_load_meta(metaimage_path, dicom_path): meta = load_ct.load_ct(dicom_path, voxel=False) assert isinstance(meta, list) assert len(meta) > 0 assert all([isinstance(_slice, dicom.dataset.FileDataset) for _slice in meta]) > meta = load_ct.load_ct(metaimage_path, voxel=False) src/tests/test_load_ct.py:64: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ src/preprocess/load_ct.py:103: in load_ct meta = load_metaimage(mhd_pattern, voxel=voxel) src/preprocess/load_ct.py:72: in load_metaimage meta = SimpleITK.ReadImage(path) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ args = ('../images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797.mhd',) def ReadImage(*args): """ ReadImage(VectorString fileNames, itk::simple::PixelIDValueEnum outputPixelType) -> Image ReadImage(std::string const & filename, itk::simple::PixelIDValueEnum outputPixelType) -> Image ReadImage is a procedural interface to the ImageSeriesReader class which is convenient for most image reading tasks. Note that when reading a series of images that have meta-data associated with them (e.g. a DICOM series) the resulting image will have an empty meta-data dictionary. It is possible to programmatically add a meta-data dictionary to the compounded image by reading in one or more images from the series using the ImageFileReader class, analyzing the meta-dictionary associated with each of those images and creating one that is relevant for the compounded image. See: itk::simple::ImageFileReader for reading a single file """ > return _SimpleITK.ReadImage(*args) E RuntimeError: Exception thrown in SimpleITK ReadImage: /tmp/SimpleITK/Code/IO/src/sitkImageReaderBase.cxx:82: E sitk::ERROR: Unable to determine ImageIO reader for "../images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797.mhd" /usr/local/lib/python3.6/site-packages/SimpleITK/SimpleITK.py:8332: RuntimeError ___________________________________________ test_metadata ___________________________________________ metaimage_path = '../images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797' dicom_path = '../images/LIDC-IDRI-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.298806137288633453246975630178/1.3.6.1.4.1.14519.5.2.1.6279.6001.179049373636438705059720603192' def test_metadata(metaimage_path, dicom_path): meta = load_ct.load_ct(dicom_path, voxel=False) meta = load_ct.MetaData(meta) zipped = zip(meta.spacing, (0.703125, 0.703125, 2.5)) assert all([m_axis == o_axis for m_axis, o_axis in zipped]) > meta = load_ct.load_ct(metaimage_path, voxel=False) src/tests/test_load_ct.py:74: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ src/preprocess/load_ct.py:103: in load_ct meta = load_metaimage(mhd_pattern, voxel=voxel) src/preprocess/load_ct.py:72: in load_metaimage meta = SimpleITK.ReadImage(path) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ args = ('../images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797.mhd',) def ReadImage(*args): """ ReadImage(VectorString fileNames, itk::simple::PixelIDValueEnum outputPixelType) -> Image ReadImage(std::string const & filename, itk::simple::PixelIDValueEnum outputPixelType) -> Image ReadImage is a procedural interface to the ImageSeriesReader class which is convenient for most image reading tasks. Note that when reading a series of images that have meta-data associated with them (e.g. a DICOM series) the resulting image will have an empty meta-data dictionary. It is possible to programmatically add a meta-data dictionary to the compounded image by reading in one or more images from the series using the ImageFileReader class, analyzing the meta-dictionary associated with each of those images and creating one that is relevant for the compounded image. See: itk::simple::ImageFileReader for reading a single file """ > return _SimpleITK.ReadImage(*args) E RuntimeError: Exception thrown in SimpleITK ReadImage: /tmp/SimpleITK/Code/IO/src/sitkImageReaderBase.cxx:82: E sitk::ERROR: Unable to determine ImageIO reader for "../images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797.mhd" /usr/local/lib/python3.6/site-packages/SimpleITK/SimpleITK.py:8332: RuntimeError =============================== 4 failed, 22 passed in 28.94 seconds ================================ ``` ## Steps to Reproduce 1. `git fetch --all` 2. `git reset --hard origin/master` 3. `sudo git clean -x -i -d` 4. `sudo docker-compose -f local.yml build && sudo bash tests/test_docker.sh` The tests fail with the aforementioned error. I'm running Ubuntu 16.04 and docker-compose version 1.15.0, build e12f3b9. Is anyone else experiencing this issue? I'm really confused since the tests pass on Travis... ## Checklist before submitting - [X] I have confirmed this using the officially supported Docker Compose setup using the `local.py` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [X] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [X] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [X] I filled out all the relevant sections of this template

Nice idea @lamby . I implemented that in c3c626f

Thanks for filing the issue @vlavorini . I think I resolved it by adding the missing content to `__init__.py`, which has been added for the identify and segment algorithm folders but not for the classify one. Can you pull my branch `123_attribute_error` and verify whether it works for you there as well?

I added the missing content of __init__.py ## Reference to official issue This addresses #123 . ## How Has This Been Tested? I rebuilt and fired up `docker-compose` again, which showed be the correct API landing page. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@antonow This sounds great since I'm a giant novice when it comes to Vue :) I removed my front-end code again and refactored the back-end code. Travis is still making some troubles although all tests pass locally ...

I adapted the source code of [Julian de Wit](https://github.com/juliandewit/kaggle_ndsb2017) whose solution scored 2nd pace during the DSB 2017. I decided to use it as it was already written for Python 3, has a manageable code base and immediately ran using my CPU only. The algorithm is documented [here](https://github.com/concept-to-clinic/concept-to-clinic/blob/master/docs/algorithm_de_wit.md). It uses a Conv3D network to detect nodules. The network returns a set of nodules characterized by their position and the probability, that the marked tissue really is a nodule. This output is wrapped into the placeholder function that's already implemented in [prediction/src/algorithms/identify/trained_model/predict](https://github.com/concept-to-clinic/concept-to-clinic/blob/master/prediction/src/algorithms/identify/trained_model.py#L11-L38). ## Description I took most of the code from steps 2 (preprocessing) and 4 (predicting) of De Wits repository as well as the models. The models, that were trained on different data sets with different data transformations such as magnification, are saved using Git LFS. So far, I kept most of the commented code so it's easier to compare this ported version with his code, especially since his implementation is not very well documented what makes it difficult to understand why certain code regions are important. ## Reference to official issue This addresses #1 ## Motivation and Context We need a possibility to automatically find nodules so we can calculate further whether they are malicious or not. I believe that it might be even better to have multiple algorithms that detect nodules and their malice since this is supposed to be the core feature of this whole project. ## How Has This Been Tested? It takes approximately 8 Minutes to detect nodules of a full DICOM scan using CPU only with my i7. Thus, I didn't want to write a test (as it probably would take days for that to finish on Travis) so I can only provide a screenshot of the output of the API endpoint:  I'd highly appreciate any feedback how to further simplify and beautify the code. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Thanks for the feedback @lamby! I already started to implement it. However, I'm a bit blocked by the fact that currently the Vue.js front-end neither has a router and other pages than the "Open Image" page, nor does jQuery seem to work (when trying to run `$('select[name=lung_orientation]').val()`, I'm getting the error `'$' is not defined`, when trying it with `window.$...` I'm getting a `window.$ is not a function`). @isms do you think you could have a look into that? I'm a complete novice when it comes to Vue :/ Otherwise I'd be supposed to wait for #110 to be completely implemented.

Hey @rahit could you give a complete piece of code that throws the error, please? That would make it a bit easier to reproduce the error I think :)

I added an initial documentation of the Therapixel algorithm. The algorithm scored 5th place during the DSB 2017. Unfortunately, there is no technical report or blog post about the approach so far. Thus, I had to derive the algorithm design from the source code and black-box models only, which resulted in a shorter documentation than for other algorithms. ## Reference to official issue This pull request addresses #23 ## Motivation and Context At some point we need to implement an algorithm that can segment the nodules and detect whether they are malicious or not. It makes sense to first evaluate the top 10 solutions for the Data Science Bowl 2017 regarding how performant they are and how easy it would be to use them in our application. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@lamby Thanks again for suggesting some really nice and clean shortcuts. I implemented them and squashed the commits.

As stated [here](https://github.com/concept-to-clinic/concept-to-clinic/pull/101#discussion_r138825291), it would be nice to automatically add the current page's CSRF token to a POST request by jQuery. ## Expected Behavior When posting to the API, one is not required to always manually parse the CSRF token from the current page and send it to the backend. ## Current Behavior One is supposed to grap the token from the page every time one wants to post something: ``` update: function(nodule) { this.$http.post(nodule.url + "update", { csrfmiddlewaretoken: $("input[name=csrfmiddlewaretoken]").val(), lung_orientation: $("select[name=lung_orientation]").val() }).then(function(response) { console.log(response); } ``` We don't want to always manually add `csrfmiddlewaretoken: $("input[name=csrfmiddlewaretoken]").val()`. ## Possible Solution Use one of jQuerys global AJAX handlers like [.ajaxSend()](https://api.jquery.com/ajaxSend/). ## Checklist before submitting - [X] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [X] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [X] I filled out all the relevant sections of this template

@isms Works like a charm now. Thank you!

@isms I deleted all node_modules folders, removed all containers and built them from scratch. I'm still running into this issue. Can you reproduce it?

Currently, when attempting to build the project using `docker-compose -f local.yml build && docker-compose -f local.yml up`, the Vue container dies while trying to set up the npm dependencies. ## Expected Behavior Building and firing up the containers should be enough to let the Vue container serve at http://localhost:8080. ## Current Behavior Firing up the Vue container fails with `Error: Cannot find module 'chalk'`: ``` ➜ concept-to-clinic git:(master) ✗ sudo docker-compose -f local.yml build && sudo docker-compose -f local.yml up [...] vue_1 | npm info it worked if it ends with ok vue_1 | npm info using npm@5.3.0 vue_1 | npm info using node@v8.5.0 vue_1 | npm info lifecycle c2c-vue@1.0.0~predev: c2c-vue@1.0.0 vue_1 | npm info lifecycle c2c-vue@1.0.0~dev: c2c-vue@1.0.0 vue_1 | vue_1 | > c2c-vue@1.0.0 dev /app/frontend vue_1 | > node build/dev-server.js vue_1 | vue_1 | module.js:529 vue_1 | throw err; vue_1 | ^ vue_1 | vue_1 | Error: Cannot find module 'chalk' vue_1 | at Function.Module._resolveFilename (module.js:527:15) vue_1 | at Function.Module._load (module.js:476:23) vue_1 | at Module.require (module.js:568:17) vue_1 | at require (internal/module.js:11:18) vue_1 | at Object.<anonymous> (/app/frontend/build/check-versions.js:1:75) vue_1 | at Module._compile (module.js:624:30) vue_1 | at Object.Module._extensions..js (module.js:635:10) vue_1 | at Module.load (module.js:545:32) vue_1 | at tryModuleLoad (module.js:508:12) vue_1 | at Function.Module._load (module.js:500:3) vue_1 | npm info lifecycle c2c-vue@1.0.0~dev: Failed to exec dev script vue_1 | npm ERR! code ELIFECYCLE vue_1 | npm ERR! errno 1 vue_1 | npm ERR! c2c-vue@1.0.0 dev: `node build/dev-server.js` vue_1 | npm ERR! Exit status 1 vue_1 | npm ERR! vue_1 | npm ERR! Failed at the c2c-vue@1.0.0 dev script. vue_1 | npm ERR! This is probably not a problem with npm. There is likely additional logging output above. vue_1 | npm WARN Local package.json exists, but node_modules missing, did you mean to install? vue_1 | vue_1 | npm ERR! A complete log of this run can be found in: vue_1 | npm ERR! /root/.npm/_logs/2017-09-13T16_15_20_703Z-debug.log ``` ## Possible Solution When I manually cd into frontend, install the dependencies using `npm install` and run `docker-compose -f local.yml up` again, it works. ## Steps to Reproduce Check out the current master `git reset --hard origin/master` Remove all files and folders that are not explicitly tracked by Git (such as npm build files or modules) `sudo git clean -x -i -d` Build and start the docker containers `sudo docker-compose -f local.yml build && sudo docker-compose -f local.yml up` You should already see that the Vue container throws an error and exits. When visiting http://localhost:8080/, you'll get a `Connection refused`. Thus, we need to install the npm dependencies manually `cd interface/frontend && npm install` Run `sudo docker-compose -f local.yml up` and visit localhost:8080 again - it should work now. I'd appreciate any feedback - maybe I made a mistake while setting it up? ## Checklist before submitting - [X] I have confirmed this using the officially supported Docker Compose setup using the `local.py` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [X] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [X] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [X] I provided **screenshots** where appropriate - [X] I filled out all the relevant sections of this template

I replaced JsonResponses by Responses which follows the DRF idioms. ## Reference to official issue This addresses #111 . ## How Has This Been Tested? I added two tests so all functions, that previously used JsonResonses, are tested whether they still return valid JSON. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Thanks @isms I wasn't aware of that. Now, ModelSerializers are used for the JSON serialization. However, so far I haven't found a solution to let the prettified JSON be rendered in a pretty way using a default HTML Renderer. Currently, I'm supposed to use a custom Renderer that wraps the JSON with `<pre>` tags. In case that's still not correct, could you point me to the correct idiomatic way to solve this, please?

@reubano I guess this one can be closed?

@antonow Thanks for the clarification! Sorry, I did not notice that conversation. So it might make sense to file a new issue about porting the front-end we already had with Django templates to Vue (@isms @lamby @reubano )? Otherwise, the implementation of the remaining front-end issues #30 #33 #34 might become problematic

@reubano Alright, I squashed everything including the work of @timothyman into one commit.

@timothyman Thanks a lot :) It looks way nicer with such detailed dependency versions! Oh I see :D I also didn't manage to run an algorithm so far because my internet connection always aborts before I finish downloading the whole training sets ...

@timothyman Do you want to add any insights you talked about in PR #99 ? We'd highly appreciate it :)

I added an initial documentation of the Julian de Wit algorithm. The algorithm output was ensembled with the results of the [Daniel Hammack algorithm](https://github.com/concept-to-clinic/concept-to-clinic/blob/master/docs/algorithm_hammack.md). ## Reference to official issue This pull request addresses #20 ## Motivation and Context At some point we need to implement an algorithm that can segment the nodules and detect whether they are malicious or not. It makes sense to first evaluate the top 10 solutions for the Data Science Bowl 2017 regarding how performant they are and how easy it would be to use them in our application. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Currently, when attempting to build the new npm front-end, I'm running into a few issues. Firstly, it's not enough anymore to run `docker-compose -f local.yml build && docker-compose -f local.yml up`. Now, one also needs to run `npm install` and `npm run build` manually in the `frontend` directory. Without these commands, the front-end could not be served. Unfortunately, the latter command throws an error while building (which is easy to fix). However, even after fixing it I can only see the navigation but no content. ## Expected Behavior The documentation says that building the docker containers and starting them should be enough to run the application locally. One would also expect to see the content we already implemented until now. ## Current Behavior One also has to run some npm commands manually, which throws an error. Even after fixing that, only the navbar can be seen but no content like we had it before with the Django templates. ## Possible Solution - move the npm commands in the Dockerfiles (might be cleaner than adding them to the documentation) - adapt the image path that's throwing an error during `npm run build` ## Steps to Reproduce Check out the current master `git reset --hard origin/master` Remove all files and folders that are not explicitly tracked by Git (such as npm build files or modules) `sudo git clean -x -i -d` Build and start the docker containers `sudo docker-compose -f local.yml build && sudo docker-compose -f local.yml up` When visiting http://localhost:8000/, you'll get a `TemplateNotFoundError`.  Thus, we need to install the npm dependencies manually and compile the application `cd interface/frontend && npm install && npm run build` You should get the error `Module not found: Error: Can't resolve '../assets/c-logo-light.svg' in '/app/frontend/src/components'`.  This can be solved by adapting the image path (only a hot fix): ``` --- a/interface/frontend/src/components/AppNav.vue +++ b/interface/frontend/src/components/AppNav.vue @@ -1,6 +1,6 @@ <template> <nav class="navbar navbar-toggleable-md navbar-inverse bg-inverse fixed-top"> - <img src="../assets/c-logo-light.svg"/> + <img src="../assets/images/c-logo-light.svg"/> <a class="navbar-brand" href="#">Concept To Clinic</a> ``` `npm run build` should now succeed. However, when visiting http://localhost:8000/, I can only see the navbar without any content when switching tabs:  ## Checklist before submitting - [X] I have confirmed this using the officially supported Docker Compose setup using the `local.py` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [X] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [X] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [X] I provided **screenshots** where appropriate - [X] I filled out all the relevant sections of this template

Currently, two tests on the master branch are failing which seemingly has been introduced by PR #97 . It would be nice if the tests would pass again. ## Expected Behavior All tests pass. ## Current Behavior 2 tests don't pass: ``` ERROR: test_landing (backend.static.tests.SmokeTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/app/backend/static/tests.py", line 7, in test_landing url = reverse('static:open-image') File "/usr/local/lib/python2.7/dist-packages/django/urls/base.py", line 91, in reverse return force_text(iri_to_uri(resolver._reverse_with_prefix(view, prefix, *args, **kwargs))) File "/usr/local/lib/python2.7/dist-packages/django/urls/resolvers.py", line 497, in _reverse_with_prefix raise NoReverseMatch(msg) NoReverseMatch: Reverse for 'open-image' not found. 'open-image' is not a valid view function or pattern name. ====================================================================== ERROR: test_get_create_image_series (backend.images.tests.SmokeTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/app/backend/images/tests.py", line 17, in test_get_create_image_series image_series, created = ImageSeries.get_or_create(uri) TypeError: unbound method get_or_create() must be called with ImageSeries instance as first argument (got str instance instead) ``` ## Steps to Reproduce Check the [Travis build of the merge commit](https://travis-ci.org/concept-to-clinic/concept-to-clinic/builds/273792503) or 1. `git reset origin/master` 2. `bash tests/test_docker.sh ` ## Possible Implementation For the first test, it might make sense to replace the 'open-image' view name with 'home', since the 'open-image' view name has been replaced with 'home' [on the current master](https://github.com/concept-to-clinic/concept-to-clinic/blob/master/interface/backend/static/urls.py#L5). ## Checklist before submitting - [X] I have confirmed this using the officially supported Docker Compose setup using the `local.py` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [X] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [X] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [X] I provided **screenshots** where appropriate - [X] I filled out all the relevant sections of this template

This should update the Travis badge in the readme to reflect this repository's current build state. ## Reference to official issue This addresses #103 ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Currently, the Travis badge that displays the build state of the latest commit on the master branch belongs to the repository concept-to-clinic/point-platform. It might make more sense that the badge should actually display the Travis state of this repository.  ## Expected Behavior The badge for this repository should be displayed:   ## Possible Solution Update the badge in the README. ## Context (Environment) The badge should be updated appropriately so we can e.g. see that currently the tests on the master branch are failing. ## Checklist before submitting - [X] I have confirmed this using the officially supported Docker Compose setup using the `local.py` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [X] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [X] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [X] I provided **screenshots** where appropriate - [X] I filled out all the relevant sections of this template

Hey @antonow thanks a lot for your contribution! Do you happen to have some time to fix the tests that are currently still failing? Otherwise it might sense to open an issue for that so we can keep the tests passing on the master :)

This PR adds the ability to select whether a nodule belongs to the right or to the left lung. ## Description I added an accordion view of all nodules with a drop-down menu to select in which part of the lung a nodule is located. ## Reference to official issue This addresses #34 . ## Motivation and Context This should be a simple entry point for adding information to a nodule. ## How Has This Been Tested? I wrote a [test](https://github.com/concept-to-clinic/concept-to-clinic/compare/master...WGierke:34_set_lung_orientation?expand=1#diff-4a8b2a0cc0727d9b77d206172538ca85R36) that checks whether POSTing to the new back-end endpoint results in an appropriately updated nodule entity. Also, I tested it manually. ## Screenshots:  ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@lamby yes it should be described in #86

I added an initial documentation of the Daniel Hammack algorithm. The algorithm output was ensembled with the results of the [Julian de Wit algorithm](https://github.com/concept-to-clinic/concept-to-clinic/issues/20) whose documentation should follow soon. Please feel free to add commits and extend it! ## Reference to official issue This pull request addresses #19 . ## Motivation and Context At some point we need to implement an algorithm that can segment the nodules and detect whether they are malicious or not. It makes sense to first evaluate the top 10 solutions for the Data Science Bowl 2017 regarding how performant they are and how easy it would be to use them in our application. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@lamby conflicts are resolved again. I'm not fooling you - after merging other pull requests into the master this branch just gets conflicts again :)

@lamby friendly ping :)

If you remove the space in front of your 'X' in the PR description, the CLA test should pass :) Could you adapt the current failing test, that checks whether the 'Open Image' page exists, to the new Vue.js frontend? I'm still not quite sure why the second test fails ...

Unfortunately, the author didn't respond so far. @reubano you already wrote that the code runs on Windows 64bit with Python 3.5. From where did you receive this information? I'm a bit confused because I found a lot `print` statements in the code that don't have parentheses which would throw errors when running with Python 3.

Hey @reubano unfortunately I had no success so far, sorry. Nevertheless, I updated the relevant sections based on your remarks.

This PR is based on and blocked by #93 (because this already adds a proper creation of candidates). To see the raw changes this PR wants to add, please see [here](https://github.com/WGierke/concept-to-clinic/compare/36_accept_reject_candidates...WGierke:30_case_json_report). Now, a JSON report can be generated for a case that includes the patient id, candidates, nodules, etc. ## Description I fixed the creation of nodules, added the JSON report and wrote a test for that. ## Reference to official issue This addresses #30. ## Motivation and Context This should summarize the data of a case and help debugging. ## How Has This Been Tested? I wrote a test which adds a case with image series, candidates and nodules and which tests for a proper report. ## Screenshots (if appropriate):  ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

And resolved :)

You're right, they've been a piece of cake - and resolved :)

@lamby Thanks, I'll stick to that in the future. The conflicts should now be resolved.

This pull request is based on #88 and also blocked by it. The buttons, that have been added in this PR, now also get a functionality (that still needs to be implemented in the back-end). ## Description You can find the "raw" changes between PR #88 and this PR [here](https://github.com/WGierke/concept-to-clinic/compare/35_list_candidates...WGierke:36_accept_reject_candidates?expand=1) which might be easier to oversee. I added two methods that are invoked when the Dismiss / Mark Concerning buttons are clicked. The functions receive the ID of the candidate as soon as the user triggers the buttons. The entire page's results are not aggregated. ## Reference to official issue This addresses #36 . ## Motivation and Context The end-user should be able to specify whether candidates might be concerning or whether they can simply be dismissed. ## How Has This Been Tested? I fixed the creation of candidates (before, there was an error stating that the serializer can't handle nested `create` calls) and created three candidates. I clicked the Dismiss and Mark buttons of candidates 1 and 3 and was ably to verify the correct API responses in the console:  ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

This pull request is based on #88 . The fetched candidates are now counted, pluralized and displayed. ## Description You can find the "raw" changes between PR #88 and this PR [here](https://github.com/WGierke/concept-to-clinic/compare/35_list_candidates...WGierke:37_candidates_found?expand=1). Filters have been removed in Vue.js 2.0 so I was supposed to import [vue2-filters](https://github.com/freearhey/vue2-filters) which includes the `capitalize` filter. This is IMO a cleaner solution than writing `capitalize` and further filters by ourselves. ## Reference to official issue This addresses #37 ## Motivation and Context The end-user should get a quick overview of the scale of the case. ## How Has This Been Tested? I used the same stubbed response as in PR #88 . The resulting screen can be seen here:  ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Do you know what the "mA" (probably milliampere?) information means and how to access it using `pydicom`?

@lamby Thanks, I squashed the commits. Hopefully, merging should work now :)

@lamby Thanks again for your feedback! I implemented it and squashed the commits.

@lamby That sounds nice :) Are there any more remarks?

This should add an "accordion" view that shows the candidate nodules for a selected image. The candidate fields `lidc_max_sensitiv, convnet_vgg, convnett_vgg_lidc, Slice` still need to be implemented in the back-end. ## Description I added the front-end Vue.js implementation to display candidate nodules of an image based on the current API. ## Reference to official issue This addresses #35 . ## Motivation and Context The doctor should be able to quickly get an overview of candidate nodules including their probability of being cancer. ## How Has This Been Tested? I created two dummy candidates by letting the API return ``` [{ "centroid": { "x": 54, "y": 78, "z": 23, "series": 0 }, "probability_concerning": 0.94, "case": 1 }, { "centroid": { "x": 1, "y": 2, "z": 3, "series": 1 }, "probability_concerning": 0.94, "case": 1 }] ``` The outcome was   ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Thanks @lamby , I added appropriate url tags and filed a new issue. What do you think of rahit's question - are we expecting that the API returns a directory structure deeper than two levels? That would most likely require us to render the file tree recursively. To resolve #31 so far I assumed that the tree should always look like the one in the picture of #31 so to only have two levels.

From [PR #85 ](https://github.com/concept-to-clinic/concept-to-clinic/pull/85#pullrequestreview-60289138): build a global JS error handler that displays a notification on any passed error. ## Expected Behavior There's one global error handler that is used as an error callback by JS functions and that displays the errors as "some kind of drop-down". ## Current Behavior Every JS function has to implement its function that handles errors. ## Detailed Description One example is `fetchData()` in `open_image.html`. Adding another method would mean to implement an error handler that should do the same thing as the one in `fetchData()`. According to the DRY principle, this logic should be moved into one component that is globally accessible. ## Possible Implementation For displaying the errors, something like [bootstrap-notify](http://bootstrap-notify.remabledesigns.com/) could be used. ## Acceptance criteria - [ ] the error handler can be invoked by scripts on different pages - [ ] it displays the error (and optimally vanishes again without any user interaction)

Thanks for your feedback! I'm not familiar with pyTorch at all but I would have never thought that it's so complicated to port it to Python 3 enable it to run on a CPU... In that case I should clearly update the relevant parts of the algorithm documentation about that. DId you happen to try how long it takes the model to predict the cancer probability on one image?

A file tree is rendered using Vue.js that should show the images the user can select from the local disk. ## Description I based this PR on rahit's work in #81 that adds a new endpoint to fetch images from. The changes render a file tree at the index page that visualizes the response from that new endpoint. ## Reference to official issue This should address #31 . ## Motivation and Context It already adds some functionality in the front-end to render the structure of the available images. This is already done in Vue.js instead of using the Django template language. ## How Has This Been Tested? `ImageAvailableApiView.get()` is currently not implemented, yet. So currently, the tree only has a lonely root:  However, assuming the view would return the following response ``` JsonResponse({'directories': [{'name': 'test_cases, no modules', 'children': ['test_case_001', 'test_case_002', 'test_case_003']}, {'name': 'test_cases, benign', 'children': ['test_case_004', 'test_case_005', 'test_case_006']}, {'name': 'test_cases, cancerous', 'children': ['test_case_007', 'test_case_008', 'test_case_009']}]}) ``` the picture referenced in issue #31 would be rendered:  ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

I implemented a basic DRY breadcrumb navigation. It's currently blocked by #73 as it's already using the basic bootstrap template. Since I don't have that much experience in front-end programming and Django, I'd be very happy about any feedback or commits. ## Description I moved the navigation to [frontend/shared/tabs.html](https://github.com/concept-to-clinic/concept-to-clinic/compare/master...WGierke:32_breadcrumb_navigation?expand=1#diff-e6833f2ee1dbb5169558392ed92ee414). Every page includes that shared template by passing the name of the current page as `active_tab`: ``` {% block tabs %} {% include "shared/tabs.html" with active_tab='detect_and_select' %} {% endblock %} ``` The navigation will then be rendered and the tab that indicates the current page should be highlighted: ``` ... <li class="nav-item {% if active_tab == 'detect_and_select' %}active{% endif %}"> <a class="nav-link" href="/detect">Detect and Select</a> </li> ... ``` ## Reference to official issue This issue addresses #32 . ## Motivation and Context The user should be able to see in the navigation on which page he currently is. ## How Has This Been Tested? I added placeholder pages and verified that when visiting every page, each tab that belongs to the page is highlighted. ## Screenshots (if appropriate):     ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

I added an initial documentation of the grt123 algorithm. Please feel free to add commits and extend it! ## Reference to official issue This pull request addresses #18 . ## Motivation and Context At some point we need to implement an algorithm that can segment the nodules and detect whether they are malicious or not. It makes sense to first evaluate the top 10 solutions for the Data Science Bowl 2017 regarding how performant they are and how easy it would be to use them in our application. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

Thanks for the template! I have a small question: where is the difference between [Prerequisites - is porting to Python 3.5 feasible?](https://github.com/concept-to-clinic/concept-to-clinic/pull/70/commits/f1aa17bd538be2166d55a7428d031e440c7d32b4#diff-946fedbc22c9d2d2db3c74954de7fd11R22) and [Adaptation into Concept to Clinic - Comment on possible problems/solutions for porting the algorithm to Python 3.5+](https://github.com/concept-to-clinic/concept-to-clinic/pull/70/commits/f1aa17bd538be2166d55a7428d031e440c7d32b4#diff-946fedbc22c9d2d2db3c74954de7fd11R105)? Should the letter one contain more detailed steps / comments?

@lamby Alright, sorry for the confusion. I merged the current master and added a screenshot of the current version (including the Git version in the top right corner).

I'd suggest you to use `flake8` here as well :) ``` ➜ concept-to-clinic git:(13_calculate_tumor_volume) flake8 prediction prediction/src/algorithms/segment/trained_model.py:53:66: W291 trailing whitespace prediction/src/algorithms/segment/trained_model.py:54:81: W291 trailing whitespace prediction/src/algorithms/segment/trained_model.py:62:77: W291 trailing whitespace prediction/src/algorithms/segment/trained_model.py:63:65: W291 trailing whitespace prediction/src/algorithms/segment/trained_model.py:65:12: W291 trailing whitespace prediction/src/algorithms/segment/trained_model.py:67:1: W293 blank line contains whitespace prediction/src/algorithms/segment/trained_model.py:76:1: W293 blank line contains whitespace prediction/src/tests/test_calculate_volume.py:17:1: W293 blank line contains whitespace prediction/src/tests/test_calculate_volume.py:24:1: W293 blank line contains whitespace prediction/src/tests/test_calculate_volume.py:35:48: W291 trailing whitespace prediction/src/tests/test_calculate_volume.py:38:1: W293 blank line contains whitespace prediction/src/tests/test_calculate_volume.py:41:47: W291 trailing whitespace prediction/src/tests/test_calculate_volume.py:43:1: W293 blank line contains whitespace prediction/src/tests/test_calculate_volume.py:46:1: E303 too many blank lines (3) prediction/src/tests/test_calculate_volume.py:52:1: W293 blank line contains whitespace prediction/src/tests/test_calculate_volume.py:59:1: W293 blank line contains whitespace prediction/src/tests/test_calculate_volume.py:64:1: W293 blank line contains whitespace ```

Thanks for your contribution! I'd suggest you to use `flake8` to check for proper PEP8 formatting of your code as it will be done by Travis as soon as Travis works again. Here is the output that was generated for me: ``` ➜ concept-to-clinic git:(2_classify_algorithm) flake8 prediction prediction/classify/trained_model.py:12:20: W291 trailing whitespace prediction/classify/trained_model.py:18:1: E402 module level import not at top of file prediction/classify/trained_model.py:18:49: W291 trailing whitespace prediction/classify/trained_model.py:21:47: W291 trailing whitespace prediction/classify/trained_model.py:39:82: W291 trailing whitespace prediction/classify/trained_model.py:64:5: E303 too many blank lines (2) prediction/classify/src/preprocess_patch.py:18:76: W291 trailing whitespace prediction/classify/src/preprocess_patch.py:29:31: W291 trailing whitespace prediction/classify/src/preprocess_patch.py:30:31: W291 trailing whitespace prediction/classify/src/preprocess_patch.py:31:31: W291 trailing whitespace prediction/classify/src/preprocess_patch.py:33:86: W291 trailing whitespace prediction/classify/src/preprocess_patch.py:34:86: W291 trailing whitespace prediction/classify/src/preprocess_patch.py:37:1: W293 blank line contains whitespace prediction/classify/src/preprocess_patch.py:45:76: W291 trailing whitespace prediction/src/tests/test_preprocess_dicom.py:2:1: F401 'os' imported but unused prediction/src/tests/test_preprocess_dicom.py:4:1: F401 'shutil' imported but unused prediction/src/tests/test_preprocess_dicom.py:19:5: F841 local variable 'voxel_shape' is assigned to but never used prediction/src/tests/test_preprocess_dicom.py:39:5: E303 too many blank lines (2) prediction/src/tests/test_preprocess_dicom.py:41:1: W293 blank line contains whitespace prediction/src/tests/test_classify_trained_model_predict.py:1:1: F401 'numpy as np' imported but unused prediction/src/tests/test_classify_trained_model_predict.py:4:1: F401 'shutil' imported but unused prediction/src/tests/test_classify_trained_model_predict.py:5:1: F401 'glob.glob' imported but unused prediction/src/tests/test_classify_trained_model_predict.py:12:1: E402 module level import not at top of file prediction/src/tests/test_classify_trained_model_predict.py:13:1: E402 module level import not at top of file prediction/src/tests/test_classify_trained_model_predict.py:28:50: W291 trailing whitespace prediction/src/tests/test_classify_trained_model_predict.py:29:42: W291 trailing whitespace prediction/src/tests/test_classify_trained_model_predict.py:30:50: W291 trailing whitespace prediction/src/tests/test_classify_trained_model_predict.py:31:61: W291 trailing whitespace prediction/src/tests/test_classify_trained_model_predict.py:38:55: W291 trailing whitespace prediction/src/tests/test_classify_trained_model_predict.py:39:53: W291 trailing whitespace prediction/src/tests/test_classify_trained_model_predict.py:42:50: W291 trailing whitespace prediction/src/tests/test_classify_trained_model_predict.py:43:69: W291 trailing whitespace prediction/src/tests/test_classify_trained_model_predict.py:44:50: W291 trailing whitespace prediction/src/tests/test_classify_trained_model_predict.py:45:67: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:13:92: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:14:62: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:15:78: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:16:65: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:18:80: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:25:17: E211 whitespace before '(' prediction/src/preprocess/preprocess_dicom.py:25:58: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:26:70: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:35:37: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:41:25: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:45:90: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:61:92: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:62:62: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:63:78: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:64:65: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:66:80: W291 trailing whitespace prediction/src/preprocess/preprocess_dicom.py:74:5: E303 too many blank lines (2) prediction/src/preprocess/preprocess_dicom.py:82:5: E303 too many blank lines (2) prediction/src/preprocess/preprocess_dicom.py:89:1: W293 blank line contains whitespace prediction/src/preprocess/preprocess_dicom.py:101:78: W291 trailing whitespace prediction/src/preprocess/load_dicom.py:42:92: W291 trailing whitespace prediction/src/preprocess/load_dicom.py:58:17: E128 continuation line under-indented for visual indent prediction/src/preprocess/load_dicom.py:59:1: W293 blank line contains whitespace ```

I added the auto-generated .rst files. ## Description I removed `_apidoc_{prediction, interface}` from `.gitignore`, removed the underscore prefix and updated the references to the folders accordingly. ## Reference to official issue This should fix #79 ## Motivation and Context This should enable us to see the code documentation on http://concept-to-clinic.readthedocs.io/en/latest/ as well. ## How Has This Been Tested? I ran `$ docs: make html && python -m http.server` and saw the documentation on `localhost:8000`. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

When visiting http://concept-to-clinic.readthedocs.io/en/latest/, the auto-generated code documentation should be visible. ## Current Behavior It is not visible.  ## Possible Solution I guess the problem is that readthedocs needs the auto-generated .rst files that are located in `_apidoc_{prediction, interface}` which are currently ignored by Git. ## Steps to Reproduce Visit http://concept-to-clinic.readthedocs.io/en/latest/ and search for the "Code Documentation" section which has been added with #8 . ## Possible Implementation Remove `_apidoc_interface` and `_apidoc_prediction` from `.gitignore`. ## Checklist before submitting - [X] I have confirmed this using the officially supported Docker Compose setup using the `local.py` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [X] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [X] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [X] I provided **screenshots** where appropriate - [X] I filled out all the relevant sections of this template

I've reached out to the author since he mentions in the README that he knows how to simplify and speed-up his code without major performance loss.

@lamby This way we are independent of third-party servers and can be sure that Bootstrap/JQuery/... are really always served (with the specific version we expect) and you can also develop locally. Though, when checking out [this SO thread](https://stackoverflow.com/questions/3832446/benefits-vs-pitfalls-of-hosting-jquery-locally) it appears to be faster including the assets from third-party servers because the browser can only open a small amount of connections at the same time to the same domain. What do you think? Should I change it again so we load the assets from external servers for better performance?

Get or create an `ImageSeries` instance given the path to a DICOM directory. ## Description I added the class function `get_or_create` to the `ImageSeries` class. ## Reference to official issue This should solve #39 ## Motivation and Context The `ImageSeries` model needs to know the data it's holding. ## How Has This Been Tested? I wrote a test for it. ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

This should add a simple and extendable Django base template to build on further UI. ## Description I added a base template and made index.html extend it. ## Reference to official issue This should implement #9 ## Motivation and Context We'd like to start at some point with a nice and extendable UI. ## How Has This Been Tested? I fired up `docker-compose` and saw the following page  ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@lamby Sorry for the delay, I finally figured out how to solve it. Now sphinx-apidoc should be used correctly and the paths in [conf.py](https://github.com/concept-to-clinic/concept-to-clinic/pull/61/files#diff-85987f48f1258d9ee486e3191495582dR23) should also be fixed.  

Any feedback is highly appreciated, especially since I'm a complete novice concerning Docker :)

For debugging purposes it would be really nice to see in the top right corner which version is currently used. The version is the short SHA-1 commit hash. ## Description Since I didn't want to add the whole `.git` directory to the Docker container, I just added `.git/logs/HEAD` as a volume which contains historical information about branch changes etc. ## Reference to official issue This should solve #29 ## Motivation and Context The big advantage is that by just adding `HEAD` we don't have the full Git history in our container. Furthermore, every time we switch branches or commit, the `HEAD` file is synchronized with the container (as well as the Docker volumes). In the container, a Django view parses the short current commit hash out of that file and displays it. This way, when switching branches not only the code in the container is synchronized with the code at the host but also the commit hash is updated accordingly. The container can then also be deployed to production. ## How Has This Been Tested? 1. `docker-compose -f local.yml up` yields  (with the short version of the commit of this branch: a9f0e6a) 2. add an empty commit: `git commit -m "empty" --allow-empty` 3. I refreshed the page and ended up with  which was the short version of the commit hash of the empty commit (1bd5c33) ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@isms oh I'm sorry. I wasn't able to see that as I have no access to the Travis site. Are you able to cancel them? Would another possibility be to squash the commits when merging the branch?

Thanks a lot @isms , I was able to fix everything :)

Thanks a lot @isms that clearly helps :)

Hey @isms Thanks for your fast comment! Sorry for the missing screenshot but yes, I can confirm it even when I'm logged in 

When trying to see the Travis logs of a build, you end up on a Travis page stating "We couldn't find the repository concept-to-clinic/concept-to-clinic". ## Expected Behavior When trying to access the Travis logs of a build, you should be able to see the logs of the respective build. ## Current Behavior You see the described error page. ## Possible Solution Are the builds only visible to admins? If so, it might be helpful to change some access rights. ## Steps to Reproduce Check a build like [this one](https://travis-ci.com/concept-to-clinic/concept-to-clinic/builds/52855581?utm_source=github_status&utm_medium=notification) ## Context (Environment) I want to see any problems that occurred when running a Travis build. ## Detailed Description Contributors should be able to access the Travis logs so they know what went wrong. ## Checklist before submitting - [X] I have confirmed this using the officially supported Docker Compose setup using the `local.py` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [X] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [X] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [X] I provided **screenshots** where appropriate - [X] I filled out all the relevant sections of this template

## Description This pull request should add the ability to auto-generate code documentation based on Google-styled docstrings. ## Reference to official issue This should solve #8 ## Motivation and Context Instead of manually adapting the documentation to the written code and its docstrings, we can now auto-generate the Sphinx documentation based on the docstrings in the code. ## How Has This Been Tested? 1. I merged my code into #55 (so it's easier for me to update the documentation :-) ) 2. I started docker-compose 3. I saw the auto-summaries of all modules, classes and methods that have proper docstrings 4. To confirm that it really supports Google-style docstrings, I downloaded [this](http://www.sphinx-doc.org/en/stable/ext/example_google.html) sample code having lots of Google-docstrings 5. I restarted docker-compose 6. I confirmed that the docstrings of that file were added to the Sphinx documentation ## Screenshots (if appropriate): Overview  Details of the interface.backend package  One legacy method that already has a complete docstring  The Google-docstring file I added for testing purposes was rendered correctly  ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

## Description I added some details on how to clone the repo using LFS, how to track files and how to remove files from the Git history. ## Reference to official issue This should implement #16 ## Motivation and Context If possible, people should use Git LFS for big files so the repository size does not exceed the quota. ## How Has This Been Tested? I checked out #55 and altered the documentation on that branch to directly see the compiled documentation on my computer. ## Screenshots (if appropriate):  ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

I'd completely agree with dropping the iframe in favor of e.g. opening the documentation page in a new tab. This way, the user stays on https://concepttoclinic.drivendata.org while he enjoys expected browsing behavior in the documentation and by following all external links.

## Expected Behavior After cloning using Git (and without LFS), after 30 commits and without test data, the repository should only be at most a few MB in size. ## Current Behavior It's over 140 MB when cloning the master.  ## Possible Solution I guess that the reason is that in the beginning even big files were pushed using Git. When they were then transferred to Git LFS, they weren't removed from the "normal" Git history. ## Steps to Reproduce 1. Clone the repository 2. Check its size (including hidden folders such as .git) ## Context (Environment) ## Detailed Description ## Possible Implementation Would it be possible to remove those big files from the history using references like [Removing sensitive data from a repository](https://help.github.com/articles/removing-sensitive-data-from-a-repository/) or [How to remove/delete a large file from commit history in Git repository?](https://stackoverflow.com/questions/2100907/how-to-remove-delete-a-large-file-from-commit-history-in-git-repository)? Unfortunately, I'm not that familiar with LFS or history rewriting... ## Checklist before submitting - [X] I have confirmed this using the officially supported Docker Compose setup using the `local.py` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [X] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [X] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [X] I provided **screenshots** where appropriate - [X] I filled out all the relevant sections of this template

I guess this can be closed as it was implemented in #53 ?

I guess this can be closed as it was implemented in #51 ?

@isms but we can't alter them of third parties such as Github. If they set frame-ancestors to none, it's not possible to frame their content at all. So I'll agree with @alexanderrich that either opening the problematic links in a new tab using `target="_blank"` or to directly link to the readthedocs page instead of including it in an iframe are the only reasonable solutions here (from my point of view).

Hey @alexanderrich the reason is that external pages (such as https://github.com/concept-to-clinic/concept-to-clinic in your example) set the [Content-Security-Policy frame-ancestors](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors) no `none`, meaning that no third party is allowed to frame their content. Since we can't alter their CSP flags, the only solution from my POV is to open all links in our documentation that refer to such pages in a new tab using `target="_blank"` for every link tag.