search

drivendataorg / concept-to-clinic

ALCF Concept to Clinic Challenge
https://concepttoclinic.drivendata.org/
MIT License
367 stars 146 forks source link

'Import' UI element should be able to create backend `ImageSeries` model instance #145

Closed Serhiy-Shekhovtsov closed 6 years ago

Serhiy-Shekhovtsov commented 7 years ago

The button exists but the behavior has not been implemented on the backend:

Expected Behavior

User should be able to import images displayed in "Import image series" section.

Current Behavior

There is tree view for browsing file system, but there is no way to select and import file\folder. image

Steps to Reproduce

Browse http://0.0.0.0:8080 and click Import button.

Checklist before submitting

isms commented 7 years ago

This needs to be implemented on the frontend and backend.

adrianjoseph commented 7 years ago

I am new to the field of data science (currently completing a Masters Program), but I am very interested in the topic/cause. ISMS, not sure if your comment is a statement or a question, but I am curious whether the solution should indeed be bounded by delivery of working code or also is a gui/application also needed?

isms commented 7 years ago

I am curious whether the solution should indeed be bounded by delivery of working code or also is a gui/application also needed?

@adrianjoseph Not sure I understand the question?

To be clear, I mocked out the Vue component (frontend) for looking through directories to find the folder of imagery we want to import as an ImageSeries model with filesystem location and metadata stored in the database (backend).

Neither part is done (100% functional, well tested, etc) so that's why both parts need work.

adrianjoseph commented 7 years ago

Ahh

That makes more sense

Appreciate you clarifying - I got my answer.

Let me take a closer look at your code

Thanks

Adrian

On Oct 10, 2017, at 9:48 PM, Isaac Slavitt notifications@github.com wrote:

I am curious whether the solution should indeed be bounded by delivery of working code or also is a gui/application also needed?

@adrianjoseph Not sure I understand the question?

To be clear, I mocked out the Vue component (frontend) for looking through directories to find the folder of imagery we want to import as an ImageSeries model with filesystem location and metadata stored in the database (backend).

Neither part is done (100% functional, well tested, etc) so that's why both parts need work.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jeffreyb92 commented 6 years ago

Fellow person completing a Data Science program, just so I understand what I would need to do to accomplish this, at the moment there is currently no implementation of importing the selected images into the frontend? And would there need to be a conversion of sorts from a dicom image format to something that can be show on the web such as JPG or PNG?

adrianjoseph commented 6 years ago

I believe so, but I don't necessarily think that it has to be in a JPG or PNG

isms commented 6 years ago

@jeffreyb92 @adrianjoseph Hey, welcome! Just to be clear, the "import image series" has more to do with creating records in the database that keep tracks of images on the filesystem (ImageSeries database model) and then the clinical diagnosis that is based on the imagery (Case).

Other issues like #148, #149, and #150 are definitely going to need to render DICOM imagery into a web-friendly format. We should not reinvent the wheel though - a web search shows a handful of libraries that already do this!

adrianjoseph commented 6 years ago

Thanks....I have been spending a fair bit of time trying to understand VTK and working with DICOM to create visual utilities.

Given how you have described this thread, I feel like I need to look at #148, #149, and #150 some more/instead. Thanks for clarifying before I got too deep in this

Serhiy-Shekhovtsov commented 6 years ago

I would like to work on this one, if nobody started yet.

Serhiy-Shekhovtsov commented 6 years ago

We have two pull requests for this issue:

PR #233 with this approach:

  1. Click Import
  2. Preview images on local system
  3. Click Load Image
  4. Start new case

And PR #236 with the approach:

  1. Preview images on local system
  2. Start new case

And there some changes requested by @louisgv that are applicable for both of these pull requests. But I am not sure about working on them, since @vessemer may be implementing the same thing. My comment asking if anyone working on the issue, as well as my concern about collaboration over competition here was silently ignored by @vessemer. @lamby would you mind stepping in here and share your thoughts? To be clear - I am not concerned about awarded points, but about transparency and fairness. We have plenty of other tasks to do here, so what's the point in wasting our time working on same things?

vessemer commented 6 years ago

Good time of a day, @Serhiy-Shekhovtsov

Do not worry, I didn't ignore your comments at all. I've been also concerned about the right way of collaboration, once I got into this project. I've noted your question in issue #4, left unanswered. I'm also used to the situations when someone takes the issue and then does nothing. These situations prevent the normal course of work. That is why I've decided to implement it by this after waiting for a sufficient amount of time.

You wrote request about 4 days ago while this issue can easily be implemented in one evening. During this time you've made another PRs #235 and #231. I also checked your fork and found nothing related to this issue. So I decided that you're not interested in working on this issue at the moment.

I also do not think, that our PRs are waisting of time, even though only one will be accepted, 'cause it usually leads to quality improvement.
From my point of view, this discussion itself is a wasting of time.

lamby commented 6 years ago

Note that #233 was just closed (see there for details) -- is this issue therefore resolved elsewhere? :)