New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#31 Show directory tree of possible images #85

Merged
merged 1 commit into from Sep 9, 2017

Conversation

Projects
None yet
3 participants
@WGierke
Copy link
Contributor

WGierke commented Sep 1, 2017

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, assuming the endpoint would return the files in a certain structure.

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:
screenshot from 2017-09-02 00-41-58

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:
screenshot from 2017-09-02 00-40-31

CLA

  • I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
@rahit

This comment has been minimized.

Copy link
Contributor

rahit commented Sep 2, 2017

Hi @WGierke . One thing that passed on my mind when I initially started working on the issue was - whether the directory structure will be fixed or it will be dynamic.

+-- root
     |-- Level 1
          |-- Level 2 directory 1
                |-- case 001
                |-- case 002
          |-- Level 2 directory 2
                |-- case 001
                |-- case 002

Do we want to grab all the files and directories under the sub directories as well? For instances, LIDC-IDRI data does not follow this structure. Nor the test images directory. If we move to another data source (or DICOM image server which is planned for production) the structure might be different.

But, if that is not the case, next step will be very easy. What do you think about the issue? I think @lamby can help us by guiding on this. :)

@WGierke WGierke force-pushed the WGierke:31_image_directory_tree branch 2 times, most recently from 7e2b283 to 708b3fe Sep 2, 2017

},
fetchAvailableImages: function() {
var vm = this;
this.$http.get('/api/images/available').then(

This comment has been minimized.

@lamby

lamby Sep 3, 2017

Contributor

{% url … %} here? :)

this.directoriesDict = response.body;
},
function() {
// error callback

This comment has been minimized.

@lamby

lamby Sep 3, 2017

Contributor

Could you file a new issue (referencing this one) regarding investigating a global error handler? Like, some kind of drop-down that will pop up on any error so we aren't having to implement it each time...

@WGierke WGierke referenced this pull request Sep 3, 2017

Closed

Gloabl JS Error Handler #86

0 of 2 tasks complete
@WGierke

This comment has been minimized.

Copy link
Contributor

WGierke commented Sep 3, 2017

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 only have two levels).

@WGierke WGierke force-pushed the WGierke:31_image_directory_tree branch 2 times, most recently from 7d8f139 to 0ed2f28 Sep 3, 2017

@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Sep 4, 2017

are we expecting that the API returns a directory structure deeper than two levels

For now, lets go with no. MVP!

@WGierke

This comment has been minimized.

Copy link
Contributor

WGierke commented Sep 5, 2017

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

@WGierke WGierke force-pushed the WGierke:31_image_directory_tree branch from 24c3408 to 8c9a8bc Sep 5, 2017

@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Sep 5, 2017

@WGierke No further comments here, am just seeing "This branch cannot be rebased due to conflicts". If you can fix that up I'll get it "live" :)

@WGierke WGierke force-pushed the WGierke:31_image_directory_tree branch from 6a2c9ef to db4520e Sep 5, 2017

@WGierke

This comment has been minimized.

Copy link
Contributor

WGierke commented Sep 5, 2017

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

@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Sep 6, 2017

@WGierke Oh dear I'm seeing conflicts with the base branch now.

Also by "squash", please don't squash unrelated commits - it's always a bad smell when you see the word "and" in a commit message; feel free to split those bits out.

@WGierke WGierke force-pushed the WGierke:31_image_directory_tree branch from d55028a to a477f6b Sep 6, 2017

@WGierke

This comment has been minimized.

Copy link
Contributor

WGierke commented Sep 6, 2017

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

@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Sep 7, 2017

@WGierke Alas still seeing "This branch cannot be rebased due to conflicts" :) ^

@WGierke WGierke force-pushed the WGierke:31_image_directory_tree branch 2 times, most recently from daaee61 to d35a4a8 Sep 7, 2017

@WGierke

This comment has been minimized.

Copy link
Contributor

WGierke commented Sep 7, 2017

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

@WGierke WGierke force-pushed the WGierke:31_image_directory_tree branch from d35a4a8 to e2ddaa5 Sep 8, 2017

@lamby lamby merged commit d6306ff into drivendataorg:master Sep 9, 2017

2 checks passed

concept-to-clinic/cla @WGierke has signed the CLA.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Sep 9, 2017

Merged.. @WGierke Did you make that ticket re a global eror handler...?

@WGierke

This comment has been minimized.

Copy link
Contributor

WGierke commented Sep 9, 2017

@lamby yes it should be described in #86

@rahit rahit referenced this pull request Sep 11, 2017

Merged

#31 Directory List of Datasource #100

3 of 3 tasks complete

@antonow antonow referenced this pull request Sep 12, 2017

Closed

Port existing pages to frontend Vue project #110

5 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment