Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vue: implement nodule 'Annotation' UI component(s) #149

Closed
isms opened this issue Oct 5, 2017 · 13 comments

Comments

@isms
Copy link
Contributor

commented Oct 5, 2017

This issue will remain open until all the checkboxes are checked! PRs can reference this issue and will get credit even if the whole issue is not yet closed 馃憤

Note: split off from #110


Design doc reference here: http://concept-to-clinic.readthedocs.io/en/latest/design-doc.html#annotate

Mock-up:

Expected Behavior

Users should be able to annotate nodules marked concerning and classify them as concerning or not concerning.

  • Provide UI controls for adding sending general RSNA template notes for the case.
  • Provide UI controls for selecting a nodule for detail view for a single nodule and display that nodule in a corresponding image viewer.
  • In the detail view for a single nodule, provide a UI control for specific a user-supplied ground truth label for P(concerning). Also show an indication of what the predicted P(concerning) was for each nodule.
  • In the detail view, provide UI controls for the nodule specific RSNA template data outlined above.
  • In the detail view, enrich the image viewer with pan, zoom, and slide traversal for taking a closer look at and around nodules.

Mocking data

Feel free to mock data for state if it is not yet implemented in the backend API, and feel free to POST to API endpoints that don't exist yet

@isms isms changed the title Vue: implement nodule Annotation UI component(s) Vue: implement nodule 'Annotation' UI component(s) Oct 5, 2017

@louisgv

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2017

I will start working on this component, a WIP PR will be issued soon.

louisgv pushed a commit to louisgv/concept-to-clinic that referenced this issue Oct 9, 2017
louisgv pushed a commit to louisgv/concept-to-clinic that referenced this issue Oct 9, 2017
@louisgv

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2017

@isms How can I obtain need a mock sample json for a nodule?

Or the better question would be the documentation for the json schema that would be used to describe a nodule

The /api/nodules.json endpoint is also returning an empty array.

image

@louisgv

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2017

Also, it seems to me that there is already a NoduleList component in charge of rendering the list of Nodule to be examine, it is however seems to be lacking most of the UI desired in the mock.

Should I implement those feature as a separate component and attach them to the Nodule.vue component, or should I refactor the Nodule list to render instead an Annotate component?

louisgv pushed a commit to louisgv/concept-to-clinic that referenced this issue Oct 9, 2017
@isms

This comment has been minimized.

Copy link
Contributor Author

commented Oct 10, 2017

@louisgv Here's how I can get some data fixtures:

First use the testing fixture factories to create some data in your local database:

docker-compose -f local.yml run interface python manage.py shell_plus

Python 3.6.2 (default, Sep  8 2017, 06:15:13) 
[GCC 4.9.2] on linux
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from backend.cases.factories import *
>>> case = CaseFactory()
>>> candidates = CandidateFactory.create_batch(5, case=case)
>>> nodule0 = NoduleFactory(candidate=candidates[0])
>>> nodule1 = NoduleFactory(candidate=candidates[1])
>>> nodule4 = NoduleFactory(candidate=candidates[4])
[exit]

Then run the API: docker-compose -f local.yml up interface

And if you go to http://[IP of your docker, could be localhost]:8000/api/ you can play with the data you just created:

image

For instance, here's the list of nodules we just created:

[
    {
        "url": "http://localhost:8000/api/nodules/1/",
        "centroid": {
            "id": 6,
            "x": 324,
            "y": 285,
            "z": 59,
            "series": 6
        },
        "created": "2017-10-10T22:16:35.620318Z",
        "lung_orientation": 0,
        "case": "http://localhost:8000/api/cases/2/",
        "candidate": "http://localhost:8000/api/candidates/1/"
    },
    {
        "url": "http://localhost:8000/api/nodules/2/",
        "centroid": {
            "id": 7,
            "x": 49,
            "y": 227,
            "z": 31,
            "series": 7
        },
        "created": "2017-10-10T22:16:44.436150Z",
        "lung_orientation": 0,
        "case": "http://localhost:8000/api/cases/3/",
        "candidate": "http://localhost:8000/api/candidates/2/"
    },
    {
        "url": "http://localhost:8000/api/nodules/3/",
        "centroid": {
            "id": 8,
            "x": 242,
            "y": 339,
            "z": 22,
            "series": 8
        },
        "created": "2017-10-10T22:16:50.403528Z",
        "lung_orientation": 0,
        "case": "http://localhost:8000/api/cases/4/",
        "candidate": "http://localhost:8000/api/candidates/5/"
    }
]
@louisgv

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2017

@isms Thank you for the instruction! Can you also take a look at my second question? I quoted them below:

It seems to me that there is already a NoduleList component in charge of rendering the list of Nodule to be examine, it is however seems to be lacking most of the UI desired in the mock. Should I implement those feature as a separate component and attach them to the Nodule.vue component, or should I refactor the Nodule list to render instead an Annotate component?

@isms

This comment has been minimized.

Copy link
Contributor Author

commented Oct 11, 2017

@louisgv I'm not sure I understand the specific question, but I guess in general I would say just do it the way that seems best - cleanest, most logical separation of concerns, DRY, etc.

If that means changing or refactoring an existing component, then by all means so so. If it means adding a new one, do that. It could also be both.

@louisgv

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2017

@isms sounds good. I was concerned that the architect who initialized the vue project might already have an idea/plan on how to implement this component. But if I can just follow the best practice/standard, then I will be on my way without having to wait for the original architect approval.

@louisgv

This comment has been minimized.

Copy link
Contributor

commented Oct 17, 2017

Update: Sorry for the inactivity. Mid-term is coming up so I couldn't work on this issue for the past week. My midterms will be finished by next week so I should be able to resume the implementation and submit the PR before the end of the month.

louisgv pushed a commit to louisgv/concept-to-clinic that referenced this issue Oct 18, 2017
Adding an add-on-editor slot to Nodule component. This allow Nodule t鈥
鈥 render external slot if need-be, in this case, it is used to render Annotate from NoduleList drivendataorg#149
@louisgv

This comment has been minimized.

Copy link
Contributor

commented Oct 18, 2017

Some progress:
image

louisgv pushed a commit to louisgv/concept-to-clinic that referenced this issue Oct 18, 2017
@louisgv

This comment has been minimized.

Copy link
Contributor

commented Oct 18, 2017

Looks slicker now:
image

@louisgv

This comment has been minimized.

Copy link
Contributor

commented Oct 18, 2017

@isms PR submitted. Due to the json for each nodule does not include the data found in the annotation editor, I simply post what I have. Tweaking them should be no problem at all when the backend is solidified.

lamby added a commit that referenced this issue Oct 19, 2017
lamby added a commit that referenced this issue Oct 19, 2017
Adding an add-on-editor slot to Nodule component. This allow Nodule t鈥
鈥 render external slot if need-be, in this case, it is used to render Annotate from NoduleList #149
lamby added a commit that referenced this issue Oct 19, 2017
@lamby

This comment has been minimized.

Copy link
Contributor

commented Oct 19, 2017

Merged :)

@lamby lamby closed this Oct 19, 2017

@isms

This comment has been minimized.

Copy link
Contributor Author

commented Oct 19, 2017

Reopening - many of the checkboxes are not actually finished.

EDIT- On second thought, closing and will reopen separate (smaller) issues.

@isms isms reopened this Oct 19, 2017

@isms isms closed this Oct 19, 2017

@vessemer vessemer referenced this issue Oct 31, 2017
1 of 1 task complete
@louisgv louisgv referenced this issue Nov 19, 2017
2 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.