Newsfeed

@isms commented on issue #134: Global CSRF Protection

Concur that CSRF is an open question, but before we move forward some thoughts: ### Status quo = we actually are using CSRF tokens The issue says: > We're not using CSRF tokens. ... but that's not really true. The backend (final arbiter of any requests) is currently using and will currently insist on CSRF tokens for `PUT`, `POST`, `PATCH` because the `CsrfViewMiddleware` [is installed](https://github.com/concept-to-clinic/concept-to-clinic/blob/master/interface/config/settings/base.py#L58). The *frontend* is not yet using these tokens. Okay, so the problem is actually just getting tokens from the backend to the frontend since the normal Django use case is to embed these somewhere in a page where there is form. **But** we're using DRF, not templates/forms, so how to we get these tokens to the frontend making API requests? One has to assume this is a fairly well understood issue with Django Rest Framework. Here's what the DRF docs [say on this](http://www.django-rest-framework.org/topics/ajax-csrf-cors/): > In order to make AJAX requests, you need to include CSRF token in the HTTP header, [as described in the Django documentation](https://docs.djangoproject.com/en/stable/ref/csrf/#ajax). ### Auth and security are sort of out of scope, but if they're not we should do auth properly Design doc expands on this, and CSRF as a threat vector does not at all make sense given the use case in the design doc. But for the purposes of argument let's set this aside. Given the "story" of the app, *if we are going to implement auth/security* then we should probably just use authentication (i.e. only privileged users get to make special requests with `TokenAuthentication`) and set the auth token locally in an env var or something. DRF [docs on token authentication](http://www.django-rest-framework.org/api-guide/authentication/#tokenauthentication) and helpful [blog post](http://kylebebak.github.io/post/django-rest-framework-auth-csrf). Thoughts @lamby @WGierke?
5 hours ago

@WGierke commented on PR #133: #120 Lung Segmentation (Hounsfield based)

@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".
5 hours, 53 minutes ago

@WGierke commented on PR #101: #34 Select Lung Orientation

@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?
6 hours, 4 minutes ago

@WGierke created a new issue: #134: Global CSRF Protection

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
6 hours, 6 minutes ago

@isms commented on PR #133: #120 Lung Segmentation (Hounsfield based)

@WGierke Simple (dumb?) idea for testing accuracy here: what if you used this segmentation to create a mask for every single image in the LIDC-IDRI data and then looked at what % of identified nodules fell inside the mask vs. outside the mask? NB, this wouldn't be part of the ordinary test suite because it assumes that all the images are available but it would be helpful to know *for science* :microscope:
6 hours, 10 minutes ago
7 hours, 13 minutes ago

@reubano commented on PR #132: Data Generator

I'm having trouble figuring out what issue this addresses. Ideas @vessemer @lamby?
8 hours, 1 minute ago

@reubano commented on issue #120: Lung Segmentation

@WGierke good point... it doesn't look like that one does actually segmentation.
8 hours, 5 minutes ago

@reubano commented on issue #121: Use SimpleITK to load DICOM-images

> SimpleITK as a port from C/C++ is for sure not really pythonic and accessing metadata is for sure a pain having to use the encoded tags. My idea was to keep the number of dependencies small and to be more consistent in what packages we use for the tasks. In this case, I feel like the pro of removing a dependency don't outweigh the con of enforcing a less pythonic library. > Also SimpleITK uses C in the backend as far as I understand and might therefor be faster. Although this might be more important if we would use it to also do preprocessing/transformation with SimpleITK. I could try some benchmarks on the weekend. Don't worry about that right now. For the MVP, we aren't really worrying about speed. Be on the lookout however, as we may want to start benchmarking and optimizing later on (at which point we can reopen this issue).
8 hours, 9 minutes ago
8 hours, 14 minutes ago

@reubano commented on PR #122: Converted Team 'grt123' identification and classification algorithms

> I think the long term solution should be to move the gtr123 preprocessing into preprocess_ct. Agreed. However, I think your PR can work without that at this stage (MVP). I looked around some more and saw that `gtr123_model.predict` actually takes a `model_path` param. So the last line could be `return gtr123_model.predict(preprocessed, centroids, model_path)`. I think then, the only change would have to be [#L14-L16 here](https://github.com/concept-to-clinic/concept-to-clinic/blob/44d50164488adb9c84f1b98824854c53ccc12181/prediction/src/tests/test_classify_trained_model_predict.py#L14-L16) in `test_classify_trained_model_predict.py`.
8 hours, 18 minutes ago

@dchansen commented on PR #122: Converted Team 'grt123' identification and classification algorithms

This would work for now. I think the long term solution should be to move the gtr123 preprocessing into preprocess_ct. I will probably not have time to do that until next week though.
8 hours, 36 minutes ago

@reubano commented on PR #122: Converted Team 'grt123' identification and classification algorithms

Ok... here's my attempt. What do you think? ```python import SimpleITK as sitk from src.algorithms.classify.src import gtr123_model from src.preprocess.load_ct import load_ct, MetaData def predict(dicom_path, centroids, model_path=None, preprocess_ct=None, preprocess_model_input=None): reader = sitk.ImageSeriesReader() filenames = reader.GetGDCMSeriesFileNames(dicom_path) if not filenames: raise FileNotFoundError() reader.SetFileNames(reader.GetGDCMSeriesFileNames(dicom_path)) image, meta = reader.Execute(), None if preprocess_ct: meta = meta or load_ct(dicom_path)[1] voxel_data = preprocess_ct(image, MetaData(meta)) else: voxel_data = image if preprocess_model_input: preprocessed = preprocess_model_input(voxel_data, centroids) else: preprocessed = voxel_data return gtr123_model.predict(preprocessed, centroids) ``` Since the keras model is now gone, do you see any reason to continue to keeping `model_path`? If not, I can help you refactor it out.
8 hours, 48 minutes ago
9 hours, 4 minutes ago

@WGierke commented on issue #120: Lung Segmentation

@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 ?
9 hours, 34 minutes ago

@reubano commented on PR #122: Converted Team 'grt123' identification and classification algorithms

> The git lfs issue should be resolved now. Awesome! > I kept the previous code because I was unsure what the long-term plans were regarding the style of the project. Conceivably, this pull request could be changed to fit the style of the previous one. While I don't agree with all the choices made, it was in many ways nicer than what I contributed here. The main reason I did not follow it more closely was one of time, as I just wanted a working adaption of the gtr123 model as soon as possible. Ok, I understand. I'm taking a look to see how to integrate both versions. > I have fixed all the flake8 issues, except the complexity ones. Hopefully, I will have them fixed by the end of the day. Great!
9 hours, 59 minutes ago

@reubano commented on issue #120: Lung Segmentation

@WGierke hmm, it looked to me like #132 addressed it. Was I mistaken?
10 hours, 4 minutes ago

@WGierke commented on issue #120: Lung Segmentation

@reubano Why has this been closed? Should I discard PR #133 now?
10 hours, 7 minutes ago
10 hours, 22 minutes ago

@dchansen commented on issue #4: Feature: Adapt the grt123 model

It was an error on my side. It's fixed in the repo now.
10 hours, 27 minutes ago

@dchansen commented on PR #122: Converted Team 'grt123' identification and classification algorithms

The git lfs issue should be resolved now. I kept the previous code because I was unsure what the long-term plans were regarding the style of the project. Conceivably, this pull request could be changed to fit the style of the previous one. While I don't agree with all the choices made, it was in many ways nicer than what I contributed here. The main reason I did not follow it more closely was one of time, as I just wanted a working adaption of the gtr123 model as soon as possible. I have fixed all the flake8 issues, except the complexity ones. Hopefully, I will have them fixed by the end of the day.
10 hours, 27 minutes ago

@lamby commented on PR #101: #34 Select Lung Orientation

@WGierke Thanks for looking into this. I knew the Django parts (and the associate jQuery global blah) but didn't know about the Vue part - thanks! > 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? Haha, absolutely. Please open another issue at the very least and copy-paste what you wrote here into there? Of course, free free to come up with a PR too.. ;) (AIUI it *was* blocking this...?)
10 hours, 33 minutes ago

@tdraebing commented on issue #121: Use SimpleITK to load DICOM-images

SimpleITK as a port from C/C++ is for sure not really pythonic and accessing metadata is for sure a pain having to use the encoded tags. My idea was to keep the number of dependencies small and to be more consistent in what packages we use for the tasks. Also SimpleITK uses C in the backend as far as I understand and might therefor be faster. Although this might be more important if we would use it to also do preprocessing/transformation with SimpleITK. I could try some benchmarks on the weekend.
11 hours, 2 minutes ago

@reubano commented on PR #122: Converted Team 'grt123' identification and classification algorithms

Looking pretty good! Just a few questions for you. I noticed that in `classify/trained_model/predict`, you commented out the previous code instead of deleted it. Was just curious, why did you decided to keep it around? Also there's still some spacing errors. Can you run `flake8 prediction` in your terminal? Finally, I see this travis error: ```bash $ git lfs pull Git LFS: (540 of 540 files, 2 skipped) 497.08 MB / 497.08 MB, 41.20 MB skipped [eb2c037dd55e2f49da95657f7a7851cbcfe2f2b516848ed03f8c5c820f3e16b4] Object does not exist on the server: [404] Object does not exist on the server [3a46182c1c4883e170d5b576d3a2645309f34817c53c16d4f7f29fbdcf799308] Object does not exist on the server: [404] Object does not exist on the server ``` I'll try to pinpoint which files exactly are missing. Meanwhile, can you verify that the model you added to `git-lfs` was properly uploaded? Thanks for your work!
11 hours, 6 minutes ago
11 hours, 32 minutes ago

@Serhiy-Shekhovtsov commented on issue #4: Feature: Adapt the grt123 model

@dchansen I am getting this error when trying to checkout your repo: > $ git-lfs.exe smudge -- prediction/src/algorithms/classify/assets/gtr123_model.ckpt > Error downloading object: prediction/src/algorithms/classify/assets/gtr123_model.ckpt (eb2c037dd55e2f49da95657f7a7851cbcfe2f2b516848ed03f8c5c820f3e16b4) > > Smudge error: Error downloading prediction/src/algorithms/classify/assets/gtr123_model.ckpt (eb2c037dd55e2f49da95657f7a7851cbcfe2f2b516848ed03f8c5c820f3e16b4): [eb2c037dd55e2f49da95657f7a7851cbcfe2f2b516848ed03f8c5c820f3e16b4] Object does not exist on the server: [404] Object does not exist on the server Any ideas what is it and how to solve it?
11 hours, 45 minutes ago

@WGierke opened a new pull request: #133: #120 Lung Segmentation (Hounsfield based)

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. ![result1](https://user-images.githubusercontent.com/6676439/30719717-dea5964c-9f24-11e7-87d0-10111b2d72d0.gif) 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
20 hours, 57 minutes ago
21 hours, 54 minutes ago

@antonow commented on PR #125: Port existing views and add VueRouter and Axios

Yeah, I was just pointing out that the API endpoints for some of these frontend actions aren't in place yet. I don't think adding those endpoints is within the scope of this issue though.
1 day, 1 hour ago

@vessemer commented on PR #132: Data Generator

@lamby, all rebased and stashed.
1 day, 4 hours ago

@WGierke commented on PR #101: #34 Select Lung Orientation

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?
1 day, 7 hours ago

@lamby commented on PR #125: Port existing views and add VueRouter and Axios

Is this ready to roll? I see some unaddressed comments and/or commentary re. HTTP methods above? :)
1 day, 10 hours ago

@lamby commented on PR #101: #34 Select Lung Orientation

> Is it worth a large effort to implement token based auth through DRF? I'm.. not sure. Perhaps going the CSRF route is preferable overall, but I don't *quite* see what's so hard about keeping it? Just make sure we include a CSRF token on every page (or header) and let javascript always add it as a parameter? This is very very common in Django land.
1 day, 10 hours ago

@vessemer opened a new pull request: #132: classification algorithm

The data augmentation process is a very common stage for a plenty of approaches in machine learning, especially in computer vision. The majority of the solutions of the #1, #2, #3 has the similar augmentation pipeline (e.g. [grt123](https://github.com/WGierke/concept-to-clinic/blob/457b7a7528fda28ed556d234135d48595eaa13ce/docs/algorithm_grt123.md), [Therapixel](https://github.com/WGierke/concept-to-clinic/blob/d9a20abfb817642ba6d98e403ed6659446125853/docs/algorithm_therapixel.md), [Daniel Hammack](https://github.com/WGierke/concept-to-clinic/blob/a47d8f28435f612d93a8eee694ac5c82500e710f/docs/algorithm_hammack.md), etc.). 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. The worth notable example of such a generator is [Keras DataGenerator](https://keras.io/preprocessing/image/) complemented by the [thorough description](https://blog.keras.io/building-powerful-image-classification-models-using-very-little-data.html). The main advantage of the Keras approach is that it for each patch build new transformation matrix from scratch but apply it only ones in comparison with step-by-step application of `scipy.ndimage` perturbations e.g. [code of Daniel Hammack](https://github.com/dhammack/DSB2017/blob/67f6474768fa47f9be95c31aa41609e0a986d8a2/training_code/aws/data_generator_fn3.py#L126-L128) where only for a rotation around the origin two calls of affine transformations were performed. Unfortunately, the main disadvantage for us is that Keras DataGenerator doesn't work with 3D data. Thus what have I done is the extension of the DataGenerator in order to deal with volumetric data such as CT scans. I've also achieved compatibility with `scipy.ndimage` to check integrity of the methods by the trusted functions (i.e. `scipy.ndimage.zoom`, `.rotate`, etc.) which I further have used in the unit tests. Resulted speedup is about 2.5 in comparison with sequantial calls of `scipy.ndimage` functions. ## The examples of the application: ### Random Rotation ![rand_rotation_b](https://preview.ibb.co/ejkutk/rand_rotation_b.png) ![rand_rotation_full_b](https://preview.ibb.co/b3FG65/rand_rotation_full_b.png) ### Random Zoom ![rand_zoom_b](https://preview.ibb.co/khYfYk/rand_zoom_b.png) ### Random Shear ![random_shear](https://preview.ibb.co/cHLim5/random_shear.png) ### Random Shift ![rand_shift_b](https://preview.ibb.co/dOvutk/rand_shift_b.png) ### Agregated ![one](https://preview.ibb.co/jq0utk/one.png) ![two](https://preview.ibb.co/iKDfYk/two.png) ## CLA - [x] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
1 day, 15 hours ago

@isms commented on PR #101: #34 Select Lung Orientation

> Ew! Also very very hard to add it back later in my experience... Fair, but this is out of scope. We specifically put auth out of bounds here. Is it worth a large effort to implement token based auth through DRF?
2 days, 2 hours ago

@isms commented on issue #114: Always pass CSRF tokens when posting with jQuery

Hi @WGierke, looks like we have a way forward with the jQuery issues (i.e. using Vue instead) and that the [CSRF discussion](https://github.com/concept-to-clinic/concept-to-clinic/pull/101#issuecomment-330028424) is separate. Any objections to closing?
2 days, 2 hours ago

@isms created a new issue: #131: Continuous improvement of nodule classification models (see #2)

## Overview We want to continuously improve the accuracy and reliability of models developed for nodule classification. This is a continuation of #2 and will remain open indefinitely. **Note:** Substantive contributions are currently eligible for increased point awards. Design doc reference: [Jobs to be done > Detect and select > Prediction service](https://concept-to-clinic.readthedocs.io/en/latest/design-doc.html#prediction-service) ## Acceptance criteria - [ ] trained model for classification - [ ] documentation for the trained model (e.g., cross validation performance, data used) and how to re-train it *NOTE: All PRs must follow the standard PR checklist.*
2 days, 2 hours ago

@isms created a new issue: #130: Continuous improvement of centroid predictive models (see #1)

## Overview We want to continuously improve the accuracy and reliability of models developed for nodule identification. This is a continuation of #1 and will remain open indefinitely. **Note:** Substantive contributions are currently eligible for increased point awards. Design doc reference: [Jobs to be done > Detect and select > Prediction service](https://concept-to-clinic.readthedocs.io/en/latest/design-doc.html#prediction-service) ## Acceptance criteria - [ ] trained model for identification - [ ] documentation for the trained model (e.g., cross validation performance, data used) and how to re-train it *NOTE: All PRs must follow the standard PR checklist.*
2 days, 2 hours ago

@reubano commented on issue #4: Feature: Adapt the grt123 model

@Serhiy-Shekhovtsov did @dchansen recommendations help you out?
2 days, 7 hours ago

@WGierke commented on issue #126: SimpleITK: "Unable to determine ImageIO reader for ..." during local pytest

Re-cloning the repository "solved" it. What the heck. But thanks for your input @vessemer !
2 days, 7 hours ago

@WGierke commented on PR #125: Port existing views and add VueRouter and Axios

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 ```
2 days, 8 hours ago

@WGierke commented on issue #126: SimpleITK: "Unable to determine ImageIO reader for ..." during local pytest

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.
2 days, 8 hours ago

@WGierke commented on PR #118: #1 Identify Nodules: Julian de Wit Algorithm

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.
2 days, 11 hours ago

@foo-bar-baz-qux opened a new pull request: #129: Fix documentation compilation

<!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> Turned out to be pretty finicky to fix, Sphinx (apidoc) and Django can be a pain to get working together. Changes made: - Re-add the two Docker images (`compile_docs` and `documentation`). I've setup a new Docker file for both of these images that uses the requirements from the interface and prediction modules as well as the documentation-specific requirement file. - Add some required additional environment variables to allow Django to at least be importable for Sphinx's apidoc. I re-use the `compose/interface/entrypoint.sh` file from the `interface` module to setup the `DATABASE_URL` env variable that's required for django, before running the Sphinx makefile. Currently a blank string works for the URL, but I'm future-proofing in case a real URL becomes necessary for Sphinx in the future. - Fix some minor formatting issues in the docstrings which were throwing some warnings - Remove the references to `backend.static` but leave the module in as the folder still appears to be in the `master` branch. ## Reference to official issue <!--- If fixing a bug, there should be an existing issue describing it with steps to reproduce --> <!--- Please link to the issue here: --> Fixes #90. ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If adding a new feature or making improvements not already reflected in an official issue, please reference the relevant sections of the design doc --> Fix documentation compilation. ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> I've run the compilation commands in the original issue and also run the whole Docker stack. It seems to compile and serve the documentation at `http://localhost:8002` without issue. Some Sphinx warnings remain regarding unreferenced MD files that I'm assuming will be used later. ## 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
2 days, 11 hours ago
2 days, 11 hours ago
2 days, 14 hours ago

@vessemer commented on issue #126: SimpleITK: "Unable to determine ImageIO reader for ..." during local pytest

This bug has been occured because SimpleITK can't read the file (obviously), and my initial thought was that the problem is related to the broken `.mhd` file, but since it runs well on both my computers with the different environments and on Travis as well, then I've become confused. May it be because you didn't rerun dockerd and it tries to preserve its previous paths?
2 days, 23 hours ago

@Serhiy-Shekhovtsov commented on issue #4: Feature: Adapt the grt123 model

> Do you get the same error when running the base repo? Yes. > I have the full model adapted for Python3 in the pull request at #122 Great!
3 days ago

@WGierke opened a new pull request: #128: #127: Fix typo in issue template: local.py -> local.yml

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
3 days, 4 hours ago

@vlavorini created a new issue: #127: Typo in the issue template

In the checklist t the end of the template, the first comma mention 'local.py', while it should be 'local.yml'. ## 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
3 days, 4 hours ago

@dchansen commented on issue #4: Feature: Adapt the grt123 model

I have the full model adapted for Python3 in the pull request at https://github.com/concept-to-clinic/concept-to-clinic/pull/122 The Nodule identification part is extremely memory hungry. Be sure to enable the volatile setting on the Variables, and reduce the chunk size in the SplitComb.
3 days, 4 hours ago

@WGierke created a new issue: #126: SimpleITK: "Unable to determine ImageIO reader for ..." during local pytest

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
3 days, 4 hours ago

@reubano commented on issue #120: Lung Segmentation

> Perhaps, it's worth to reform the segment part, so that latter will contain lungs, nodules or something similar. Well, we can add appropriately named functions, e.g., `lung_affine_segmentation` (or whatever would be appropriate). If the `trained_model.py` file get's too large, we can always split it into 2 files later on.
3 days, 4 hours ago

@dchansen commented on PR #122: Converted Team 'grt123' identification and classification algorithms

I am a little unsure why, but the conflict between Tensorflow and Pytorch seems gone. It was realted to this issue in Tensorflow https://github.com/tensorflow/models/issues/523
3 days, 5 hours ago

@reubano commented on issue #121: Use SimpleITK to load DICOM-images

Is there anything that the `pydicom` / `dicom-numpy` duo can do that can't (easily) be done with `SimpleITK`? From briefly looking over the docs, `SimpleITK` appears to be a bit more verbose and less pythonthic. ``` import SimpleITK as sitk reader = sitk.ImageSeriesReader() dicom_names = reader.GetGDCMSeriesFileNames('/path/to/dicom/files') reader.SetFileNames(dicom_names) image = reader.Execute() reader.GetMetaData(0, '0010|0010') ``` vs ``` import dicom image = dicom.read_file('/path/to/dicom/files') ds.PatientsName ``` I also found `pydicom`'s docs a bit more comprehensive / easier to understand. Thoughts?
3 days, 7 hours ago

@reubano commented on issue #123: AttributeError: module 'src.algorithms.classify' has no attribute 'trained_model'

> 'local.py' in the checklist should be 'local.yml', right? Is there a typo somewhere in the docs? If so, mind submitting another issue about it? Thanks!
3 days, 7 hours ago
3 days, 12 hours ago

@lamby commented on PR #101: #34 Select Lung Orientation

> I suggest we disable the CSRF middleware in Django. Thoughts? Ew! Also very very hard to add it back later in my experience...
3 days, 12 hours ago

@antonow opened a new pull request: #125: Port existing views and add VueRouter and Axios

## Description This PR takes the existing views built by @WGierke, including his unmerged frontend work on [#34 Select Lung Orientation](https://github.com/concept-to-clinic/concept-to-clinic/pull/101), and turns them into components of the main Vue app. VueRouter was set up to handle page navigation, and jQuery and Bootstrap.js were removed. Axios, an alternative to jQuery's Ajax, was added because it makes it easier to set default request options. I also made a few backend changes because I was getting errors when trying to create some test data through the Django REST interface. ## Reference to official issue [Port existing pages to frontend Vue project #110](https://github.com/concept-to-clinic/concept-to-clinic/issues/110) ## Motivation and Context This change will allow us to serve all templates from the main Vue app, and sets up basic routing and frontend file organization patterns that will give the app some structure moving forward. ## How Has This Been Tested? This has been tested by running `docker-compose -f local.yml build` to install new dependencies, followed by `docker-compose -f local.yml up`, and navigating to all the links in the navigation. I created test data using the Django REST interface to verify that actions like toggling elements that can be opened and closed and making backend requests works as expected. ## Screenshots (if appropriate): <img width="998" alt="screen shot 2017-09-18 at 8 27 20 pm" src="https://user-images.githubusercontent.com/7776175/30573252-0389e8ea-9cb0-11e7-9043-34835acd6ad4.png"> <img width="996" alt="screen shot 2017-09-18 at 8 27 58 pm" src="https://user-images.githubusercontent.com/7776175/30573253-038babee-9cb0-11e7-8cdc-c0646d30c23a.png"> <img width="997" alt="screen shot 2017-09-18 at 8 28 20 pm" src="https://user-images.githubusercontent.com/7776175/30573251-0388811c-9cb0-11e7-9c64-a5bb6fb67593.png"> ## CLA - [X] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
3 days, 15 hours ago

@vessemer commented on issue #120: Lung Segmentation

@reubano, I guess you're right, it'll be convenient to place the lung segmentation method inside the `segment` directory. For now, the project has been structured in such a way that all `algorithm`'s subdirectories contain `trained_model.py`. Perhaps, it's worth to reform the `segment` part, so that latter will contain `lungs`, `nodules` or something similar. Since all stages performed prior to the classification may be roughly counted as preprocessing.
3 days, 19 hours ago

@reubano commented on issue #120: Lung Segmentation

Thanks for this @vessemer. The links you provided were very informative! I learned quite a bit just gleaming through them. My initial thought is, how much of this should be handled by #3 vs common functions that would be shared across #1, #2, and #3?
3 days, 20 hours ago

@reubano commented on issue #4: Feature: Adapt the grt123 model

@Serhiy-Shekhovtsov > Hi @reubano! I've fixed few issues on pre-processing part and managed to have it running successfully and giving the same result as original code. That's great! Nice job! > Does it really take this much memory or is it an issue with environment \ code? I'm afraid i don't know the answer to that one. Do you get the same error when running the base repo?
3 days, 23 hours ago

@vessemer commented on PR #122: Converted Team 'grt123' identification algorithm

It was my concern for the further work, I guess it's worth to define at the moment of mvp which ML backend will be used. As @reubano sed in [#2 (comment)](https://github.com/concept-to-clinic/concept-to-clinic/issues/2#issuecomment-325328384): > [...] of course, the use of TensorFlow/Theano/etc. will be up to the individual implementation. But since the grt123 algorithm can be ported on the TenserFlow backend will this approach be desired?
4 days, 1 hour ago

@dchansen commented on PR #122: Converted Team 'grt123' identification algorithm

@isms I agree that it's not a feasible compromise at all. I did it mainly to highlight the issue. I think we should have a single deep learning framework for the project as a whole, as they don't really play well together. I am not sure if Keras is the best choice, but either way, the project leads should make the decisions. Perhaps we should open an issue? Converting the model itself into Keras would relatively easy, but converting the weights is somewhat more involved. Converting to Caffe2 or CNTK would be easy, as they all support the ONNX format.
4 days, 1 hour ago

@isms commented on PR #122: Converted Team 'grt123' identification algorithm

> [...] the pull request also disables the Keras import. @dchansen I'm not sure this is a feasible compromise. Is there a way to reimplement this in Tensorflow?
4 days, 2 hours ago
4 days, 2 hours ago

@WGierke commented on issue #123: AttributeError: module 'src.algorithms.classify' has no attribute 'trained_model'

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?
4 days, 3 hours ago

@WGierke opened a new pull request: #124: #123 Fix AttributeError: module 'src.algorithms.classify' has no attribute

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
4 days, 3 hours ago

@vlavorini created a new issue: #123: AttributeError: module 'src.algorithms.classify' has no attribute 'trained_model'

I'm using the official Docker development configuration. I only cloned repository, set up the docker environment (copy-paste from the instructions), and opened the page at localhost:8001 I expected to see the sort of welcome page, but I saw just the debugger page, with the error you read in the title. A possible solution: In the file: concept-to-clinic/prediction/src/views.py change two rows: #from .algorithms import classify to from .algorithms.classify import trained_model and #'classify': classify.trained_model.predict, to 'classify': trained_model.predict, ## Steps to Reproduce 1. set up a clean environment 2. run docker-compose -f local.yml up 3. open http://localhost:8001 ## 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. - [] I provided **screenshots** where appropriate - [x] I filled out all the relevant sections of this template P.S. 'local.py' in the checklist should be 'local.yml', right?
4 days, 3 hours ago

@dchansen opened a new pull request: #122: Converted Team 'grt123' identification algorithm

<!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> This pull request implements the grt123 identification algorithm. It is based on PyTorch, which currently conflicts with Tensorflow, so the pull request also disables the Keras import. While the algorithm is quite fast on GPU, it is painfully slow (but working) on the CPU due to a bug in PyTorch. The bug is fixed in the PyTorch development branch, but the next release is about 2 months off. ## Reference to official issue <!--- If fixing a bug, there should be an existing issue describing it with steps to reproduce --> <!--- Please link to the issue here: --> https://github.com/concept-to-clinic/concept-to-clinic/issues/4 https://github.com/concept-to-clinic/concept-to-clinic/issues/1 ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If adding a new feature or making improvements not already reflected in an official issue, please reference the relevant sections of the design doc --> This Pull request implements part of the prediction service (https://concept-to-clinic.readthedocs.io/en/latest/design-doc.html#detect-prediction-service) ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> The algorithm has largely been tested by manually confirming about 5 random LUNA2016 datasets - as the loaded model has already been tested extensively elsewhere, this was deemed sufficient. It has been tested both outside of docker, on a Titan X GPU, and inside docker on an i7 Intel CPU. ## 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
4 days, 4 hours ago

@tdraebing commented on PR #119: Add function that loads MetaImage files

Awesome. Thanks for checking up on the license! I opened a new issue to deal with loading the DICOM-images with SimpleITK.
4 days, 4 hours ago

@tdraebing created a new issue: #121: Use SimpleITK to load DICOM-images

<!--- Provide a general summary of the issue in the Title above --> ## Expected Behavior <!--- Tell us what should happen --> DICOM-images and -series should be loaded using the `SimpleITK`-package. ## Current Behavior <!--- Tell us what happens instead of the expected behavior --> Currently DICOM-images are loaded into memory by using `pydicom` and `dicom-numpy` in `load_dicom.py`. A pull request ( is on its way ## Possible Solution <!--- Not obligatory, but suggest a fix/reason for the bug, --> ## Steps to Reproduce <!--- Provide a link to a live example, or an unambiguous set of steps to --> <!--- reproduce this bug. Include code to reproduce, if relevant --> 1. 2. 3. 4. ## Context (Environment) <!--- How has this issue affected you? What are you trying to accomplish? --> <!--- Providing context helps us come up with a solution that is most useful in the real world --> ## Detailed Description <!--- Provide a detailed description of the change or addition you are proposing --> ## Possible Implementation <!--- Not obligatory, but suggest an idea for implementing addition or change --> ## Checklist before submitting - [ ] 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 - [ ] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [ ] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [ ] I provided **screenshots** where appropriate - [ ] I filled out all the relevant sections of this template
4 days, 5 hours ago

@isms commented on PR #101: #34 Select Lung Orientation

@antonow concur completely on removing jQuery. Let's make the CSRF question even simpler: per the design document, security concerns are out of scope so XSS is not a concern at the moment. I suggest we disable the CSRF middleware in Django. Thoughts?
4 days, 5 hours ago

@vessemer commented on PR #119: Add function that loads MetaImage files

Hi @tdraebing, thanks for your detailed review and suggestions. I've refactored the code, all rebased and stashed. > From your tests I read that you added data from the LUNA16-competition. Did you check, whether using them in other projects is fine with their license? As it was claimed on the official page of [LUNA16/data](https://luna16.grand-challenge.org/data/) the dataset was made from LIDC/IDRI database via filtering out CT scans which slice thickness greater than 2.5 mm and may be other processing. The LIDC/IDRI database, in turn, consist of DICOM files and have [Creative Commons Attribution 3.0 Unported License](https://creativecommons.org/licenses/by/3.0/). Thus it's fine to use the LUNA16 data in a project. > Fully switching to SimpleITK would reduce the number of dependencies and make the code easier to maintain and follow. There would be also some changes in crop_dicom and preprocess_dicom needed. This might also be handled in a new issue. I guess it will be a good idea to open a new issue dedicated to that task since it will lead to a quite cumbersome refactoring. @tdraebing, would you like to open it?
4 days, 7 hours ago

@WGierke commented on PR #101: #34 Select Lung Orientation

@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 ...
4 days, 7 hours ago

@dchansen commented on PR #119: Add function that loads MetaImage files

I would strongly support moving to SimpleITK for everything, as it correctly deals with things like calibration curves, orientations, etc and gives us an easy way to pass along images and their metadata in one package.
4 days, 10 hours ago

@lamby commented on PR #118: #1 Identify Nodules: Julian de Wit Algorithm

@WGierke > 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) Good thoughts. However, what I suggest we do is to make a decorator for tests thats skip them *except* for the case where we export an environment variable such as ``RUN_SLOW_TESTS=1``. That way we can have almost the best of both worlds. Don't forget that tests also function as kind of documentation/examples on how to use an API, so we would gain that useful bit as well. :)
4 days, 10 hours ago

@antonow commented on PR #101: #34 Select Lung Orientation

@WGierke and @isms, I would like to propose removing jQuery (and bootstrap.js) from the project altogether. Vue alone is able to handle all the front-end reactivity that bootstrap does in their JS-supported elements with less code. Also, jQuery's ajax isn't well suited to setting default data params -- see the convoluted solutions described [here](https://groups.google.com/forum/#!topic/jquery-dev/OBcEfgvTJ9I). Instead we could use [axios](https://github.com/mzabriskie/axios#request-config), which makes it easy to set base configuration options including options for `xsrfTokens` and `xsrfHeaders`. Or we could use Vue's built in http functionality as @isms suggested above. Anyway, I am close to done with a PR that should address #110, and I've integrated/ported the frontend code from this PR for which I'll be sure to credit @WGierke. I hope to have that ready for review soon, but I wanted to give my opinion regarding using jQuery after reading through the most recent comments on this PR.
4 days, 13 hours ago

@tdraebing commented on PR #119: Add function that loads MetaImage files

Hi @vessemer, thanks for the pull request. I am currently pretty swamped with the last months in my PhD, so I didn't manage to finish my pull request in the last days. Based on my work on the issue, I have some further suggestions: - The `load_dicom`-function uses pydicom and dicom-numpy to load the DICOM-images. SimpleITK also has a method implemented to do so (which might even be more tolerant to compressed images). Fully switching to SimpleITK would reduce the number of dependencies and make the code easier to maintain and follow. There would be also some changes in `crop_dicom` and `preprocess_dicom` needed. This might also be handled in a new issue. - In your `MetaData`-class you calculate the slice thickness. There actually is a value for this in the metadata of the DICOM-images: '0018|0050'. (or for a pydicom-object `SliceThickness`. - The `load_meta`-function seems to be quite redundant to `load_ct`. Maybe by adding a flag (like `voxel=True` if the voxel data should be returned as well) would be better here. - This is just my personal preference, but maybe it would make sense to put the logic in `MetaData.__init__()` into a separate method(s). That would make it easier to extend the object later on. For example if patient data becomes relevant in the interface, which would also be stored in the metadata. - From your tests I read that you added data from the LUNA16-competition. Did you check, whether using them in other projects is fine with their license?
4 days, 13 hours ago

@vessemer created a new issue: #120: Lung Segmentation

Prior to all of [#1](https://github.com/concept-to-clinic/concept-to-clinic/issues/1), [#2](https://github.com/concept-to-clinic/concept-to-clinic/issues/2), [#3](https://github.com/concept-to-clinic/concept-to-clinic/issues/3) stages, preprocessing must be performed over a CT scan. Among a plenty of trivial preprocessing stages such as clipping by value, rescaling and magnitude normalisation outstands lungs segmentation task. Since the majority of the best solutions contains in there pipelines such stage, it's become important to deal with ambiguity. There exists various approaches dedicated to lungs segmentation e.g. via convenient segmentation by [value in Hounsfield scale](https://www.kaggle.com/gzuidhof/full-preprocessing-tutorial), affine/rigid/non-rigid [image registration](http://simpleelastix.readthedocs.io/GettingStarted.html), segmentation using [watershed algorithm](https://www.kaggle.com/ankasor/improved-lung-segmentation-using-watershed) etc. Approaches listed above have their pros and cons and a tradeoff as usual somewhere in between of resources consuming and a quality of segmentation. ## Expected Behavior Adopted or created lung segmentation algorithm should lie in the `src/preprocess/lungs_segmentation.py`. It also should be integrated into classes `src.preprocess.preprocess_ct.PreprocessCT` and `src.preprocess.preprocess_ct.Params`. ## Current Behavior For now `PreprocessDicom` doesn't contain any lung segmentation algorithm. ## Acceptance criteria - [ ] method aimed at lungs segmentation - [ ] brief justification of a selected approach - [ ] tests for the methods
4 days, 16 hours ago

@vessemer commented on issue #98: Add function that loads MetaImage files

As far as I saw no changes for more than the week, I've created a pull request.
4 days, 18 hours ago

@vessemer opened a new pull request: #119: Add function that loads MetaImage files

The `load_metaimage` function that load MetaImage files have been proposed along with `load_ct` ensembling method which aimed to organize loading process. Refactoring has been performed, taking into account various format of meta-information. In order to handle it standardisation via `class MetaData` has been employed, for now, it contains only one parameter (`spacing`) which is used already. Also decoupling of reading and preprocessing was included. ## Motivation and Context This addresses [#98](https://github.com/concept-to-clinic/concept-to-clinic/issues/98). The MetaImage file format is actively used by plenty of medical studies. The LUNA16 challenge is among them. Since LUNA16 was involved in kaggle DSB 2017 competition, therefore, it's into of our area of interest. ## How Has This Been Tested? The unit tests were created for each of introduced functions. ## CLA - [x] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
4 days, 18 hours ago