drivendataorg / concept-to-clinic

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

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

Closed isms closed 7 years ago

isms commented 7 years ago

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 :+1:

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.

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 :sparkles:

louisgv commented 7 years ago

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

louisgv commented 7 years ago

@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 commented 7 years ago

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?

isms commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

Some progress: image

louisgv commented 7 years ago

Looks slicker now: image

louisgv commented 7 years ago

@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 commented 7 years ago

Merged :)

isms commented 7 years ago

Reopening - many of the checkboxes are not actually finished.

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