Skip to content
This repository has been archived by the owner. It is now read-only.

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

Closed
Serhiy-Shekhovtsov opened this issue Sep 30, 2017 · 12 comments

Comments

@Serhiy-Shekhovtsov
Copy link
Contributor

@Serhiy-Shekhovtsov Serhiy-Shekhovtsov commented Sep 30, 2017

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

  • User hits 'Import' button
  • User navigates to the imagery folder and selects it with a click
  • User clicks a UI element that causes a POST payload to be sent to the API
  • API creates an ImageSeries instance with a pointer to the folder on disk and metadata as appropriate

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

  • 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
  • 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
@isms
Copy link
Contributor

@isms isms commented Oct 10, 2017

This needs to be implemented on the frontend and backend.

@isms isms modified the milestone: 1-mvp Oct 10, 2017
@adrianjoseph
Copy link

@adrianjoseph adrianjoseph commented Oct 11, 2017

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 isms added this to the 1-mvp milestone Oct 11, 2017
@isms
Copy link
Contributor

@isms isms commented Oct 11, 2017

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
Copy link

@adrianjoseph adrianjoseph commented Oct 11, 2017

@jeffreyb92
Copy link

@jeffreyb92 jeffreyb92 commented Oct 16, 2017

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
Copy link

@adrianjoseph adrianjoseph commented Oct 18, 2017

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

@isms
Copy link
Contributor

@isms isms commented Oct 18, 2017

@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
Copy link

@adrianjoseph adrianjoseph commented Oct 22, 2017

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

@isms isms changed the title Import image series 'Import' button should actually create backend ImageSeries model instance Oct 23, 2017
@isms isms changed the title 'Import' button should actually create backend ImageSeries model instance 'Import' UI element should be able to create backend ImageSeries model instance Oct 23, 2017
@isms isms modified the milestones: 1-mvp, 2-feature-building Oct 29, 2017
@Serhiy-Shekhovtsov
Copy link
Contributor Author

@Serhiy-Shekhovtsov Serhiy-Shekhovtsov commented Nov 15, 2017

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

@vessemer vessemer mentioned this issue Nov 19, 2017
1 of 1 task complete
Serhiy-Shekhovtsov added a commit to Serhiy-Shekhovtsov/concept-to-clinic that referenced this issue Nov 19, 2017
Serhiy-Shekhovtsov added a commit to Serhiy-Shekhovtsov/concept-to-clinic that referenced this issue Nov 19, 2017
Serhiy-Shekhovtsov added a commit to Serhiy-Shekhovtsov/concept-to-clinic that referenced this issue Nov 19, 2017
Serhiy-Shekhovtsov added a commit to Serhiy-Shekhovtsov/concept-to-clinic that referenced this issue Nov 19, 2017
@Serhiy-Shekhovtsov
Copy link
Contributor Author

@Serhiy-Shekhovtsov Serhiy-Shekhovtsov commented Nov 20, 2017

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
Copy link
Contributor

@vessemer vessemer commented Nov 20, 2017

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.

Serhiy-Shekhovtsov added a commit to Serhiy-Shekhovtsov/concept-to-clinic that referenced this issue Nov 20, 2017
Serhiy-Shekhovtsov added a commit to Serhiy-Shekhovtsov/concept-to-clinic that referenced this issue Nov 20, 2017
@lamby
Copy link
Contributor

@lamby lamby commented Dec 12, 2017

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

@reubano reubano closed this Dec 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants