[API] Added features for dicoms : PatientName and images: isPhantom #6899

spell00 · 2020-08-12T17:22:20Z

Brief summary of changes

Patientname is added to dicoms endpoint view and IsPhantom is added to images endpoint view

Testing instructions

Go to <hostname>/api/v0.0.3/candidates/400162/V6/dicoms/. Under the attribute Tarname, a new attribute should be there: Patientname : "ROM162_400162_V6"
Go to <hostname>/api/v0.0.3/candidates/400162/V6/images/. Each file should have 4 attributes, the last one being the new attribute, IsPhantom: false

driusan

These changes do not correspond to the API spec.

xlecours · 2020-08-26T15:03:12Z

@driusan do you mean that the spec should be adjusted in this PR? Or do you object to this change because it does not correspond to the API spec?

driusan · 2020-08-26T15:42:36Z

0.0.3 was released, any changes need to go into a new version which incorporates the changes both in the code and the spec for that version

xlecours · 2020-08-26T16:10:03Z

@driusan Additions do not compromise the specs for existing users. Managing a new version for every modification is a lot of overhead.

driusan · 2020-08-26T16:16:02Z

New additions without a new version absolutely compromises the spec. It's an API intended for third party usage. Values can't sometimes exist and sometimes not exist with the same version of the API and no way for the caller to know what it's going to get.

christinerogers · 2020-08-27T19:49:32Z

@xlecours @driusan Can we find a path forward here or is it better to mark this as Needs Discussion and we'll shelve it until the LORIS meeting?
A new version of the spec could be done but up to you guys when/where we start it from.

spell00 · 2020-09-01T20:54:03Z

The feature added in this endpoint is not in the specs for v0.0.3, so the changes suggested in this PR need to be added to the API v0.0.4 initiated in PR #6944, which is in progress.

christinerogers · 2020-10-19T22:49:35Z

@spell00 is this PR still valid and ready for re-review? If so please re-request review from driusan (click the cycling arrows in the Reviewers list)

spell00 · 2020-10-20T14:45:52Z

@christinerogers It is still blocked by PR #6944

xlecours · 2020-10-30T13:38:59Z

LGTM except it is failing travis.

spell00 · 2020-10-30T14:56:34Z

@driusan the modifications are now added to the specs of v0.0.4-dev

This reverts commit 70be45e.

This reverts commit bc711e6.

…"" This reverts commit 9bcc17c.

This reverts commit 92ffc45.

This reverts commit bc711e6.

This reverts commit 92ffc45.

spell00 · 2020-11-05T04:45:03Z

@xlecours @driusan It is now passing Travis and ready for final review

driusan · 2020-11-05T13:57:58Z

modules/api/docs/LorisRESTAPI_v0.0.4-dev.md

@@ -553,6 +553,7 @@ the form:
        "OutputType" : "native",
        "Filename" : "abc.mnc",
        "AcquisitionType" : "t1w/t2w/etc",
+        "IsPhantom" : true/false,


I was going to say this isn't valid js so it's going to be rendered as an error since it's in a JS block, but then I decided to check if it's valid js in my JS console..

>> $x = true / false Infinity

....

driusan · 2020-11-05T13:59:02Z

modules/api/php/views/visit/dicoms.class.inc

-            'Tarname'    => $dicom->getTarname(),
-            'SeriesInfo' => array_map(
+            'Tarname'     => $dicom->getTarname(),
+            'Patientname' => $dicom->getPatientname(),


This is changing the result for both 0.0.3 and 0.0.4-dev, it needs a version check.

driusan · 2020-11-05T13:59:16Z

modules/api/php/views/visit/images.class.inc

@@ -58,6 +58,7 @@ class Images
                    'OutputType'      => $image->getOutputType(),
                    'Filename'        => $image->getFilename(),
                    'AcquisitionType' => $image->getAcquisitionprotocol(),
+                    'IsPhantom'       => $image->isPhantom(),


spell00 · 2020-11-06T16:27:42Z

@xlecours @driusan I did the corrections, this is ready for re-review

driusan

Looks fine to me, @xlecours does it still look good to you after the latest changes?

xlecours · 2020-11-09T16:04:46Z

Looks fine to me, @xlecours does it still look good to you after the latest changes?

yes.
I guess this will be the way to go for new endpoint response format.

switch on version
use a class which name is suffixed by the -dev version.

Once -dev is removed, replace the file by the -dev content then delete -dev.

driusan · 2020-11-09T18:26:26Z

I don't think we're committed to that just because it's done that way in 1 place. We can always update the coding style to something more robust later, but since it works for this and the author of the PR is leaving I think we should merge it.

…ces#6899) Patientname is added to dicoms endpoint view and IsPhantom is added to images endpoint view.

spell00 force-pushed the 2020-08-11-AddFeaturesViewsFromDTO branch from fa61255 to 79cff9a Compare August 13, 2020 16:40

spell00 requested a review from xlecours August 24, 2020 16:20

spell00 changed the title ~~[API] New features to view on candidates dicoms and images endpoints~~ [API] Added features dicoms : PatientName and images: isPhantom Aug 24, 2020

spell00 requested a review from AlexandraLivadas August 24, 2020 20:47

christinerogers changed the title ~~[API] Added features dicoms : PatientName and images: isPhantom~~ [API] Added features for dicoms : PatientName and images: isPhantom Aug 24, 2020

driusan requested changes Aug 26, 2020

View reviewed changes

spell00 added API PR or issue introducing/requiring modifications to the API code Blocked PR awaiting the merge, resolution or rejection of an other Pull Request labels Sep 1, 2020

spell00 force-pushed the 2020-08-11-AddFeaturesViewsFromDTO branch from 79cff9a to 245b8dd Compare September 4, 2020 18:38

xlecours mentioned this pull request Sep 18, 2020

Transition to API version 0.0.4 #6944

Merged

spell00 added 4 commits October 29, 2020 12:05

[API] Added isPhantom to images attributes

d259d63

[API] isPHantom typo corrected

a1a7a79

[API] Add missing attributes from DicomTarDTO and ImageDTO

f06d25c

Adds new features to the integration test suite

0acacb8

spell00 force-pushed the 2020-08-11-AddFeaturesViewsFromDTO branch from ce5b2f1 to 0acacb8 Compare October 29, 2020 16:05

spell00 removed the Blocked PR awaiting the merge, resolution or rejection of an other Pull Request label Oct 29, 2020

updating v0.0.4-dev doc

3f17dbc

spell00 requested a review from driusan October 30, 2020 01:02

spell00 added 2 commits October 29, 2020 21:44

Restore last lines

39b8495

remove last commit

0f425c1

spell00 added 6 commits November 4, 2020 14:26

[API] isPHantom typo corrected

332416d

Restore last lines

087bef6

remove last commit

645bcb1

revert to previous commit 0f425c1

0a2e346

Revert "[genomic_browser] upgraded module for react (aces#5835)"

9bcc17c

This reverts commit 70be45e.

remove last thing from bad rebase

bc711e6

spell00 force-pushed the 2020-08-11-AddFeaturesViewsFromDTO branch from c8f08ee to bc711e6 Compare November 4, 2020 19:27

spell00 added 9 commits November 4, 2020 14:28

Revert "remove last thing from bad rebase"

92ffc45

This reverts commit bc711e6.

Revert "Revert "[genomic_browser] upgraded module for react (aces#5835)…

9b743a3

…"" This reverts commit 9bcc17c.

Revert "Revert "remove last thing from bad rebase""

8c4b3f4

This reverts commit 92ffc45.

Revert "remove last thing from bad rebase"

495a60d

This reverts commit bc711e6.

Revert "Revert "remove last thing from bad rebase""

f125da7

This reverts commit 92ffc45.

reverting

4d3c890

remove errors

f60443d

Removing errors from rebase

a1a1fbc

last thing to finish clearing bad rebase

dd202d3

spell00 removed the Blocked PR awaiting the merge, resolution or rejection of an other Pull Request label Nov 5, 2020

driusan requested changes Nov 5, 2020

View reviewed changes

spell00 added 2 commits November 5, 2020 13:25

add version

0c5321e

remove files

9623b9b

spell00 requested a review from driusan November 6, 2020 06:34

driusan approved these changes Nov 6, 2020

View reviewed changes

xlecours approved these changes Nov 9, 2020

View reviewed changes

driusan merged commit 70ae672 into aces:main Nov 9, 2020

ridz1208 added this to the 24.0.0 milestone Nov 27, 2020

AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 29, 2021

[API] Added features for dicoms : PatientName and images: isPhantom (a…

b34fdef

…ces#6899) Patientname is added to dicoms endpoint view and IsPhantom is added to images endpoint view.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[API] Added features for dicoms : PatientName and images: isPhantom #6899

[API] Added features for dicoms : PatientName and images: isPhantom #6899

spell00 commented Aug 12, 2020 •

edited

Loading

driusan left a comment

xlecours commented Aug 26, 2020

driusan commented Aug 26, 2020

xlecours commented Aug 26, 2020

driusan commented Aug 26, 2020 •

edited

Loading

christinerogers commented Aug 27, 2020

spell00 commented Sep 1, 2020

christinerogers commented Oct 19, 2020

spell00 commented Oct 20, 2020

xlecours commented Oct 30, 2020

spell00 commented Oct 30, 2020

spell00 commented Nov 5, 2020

driusan Nov 5, 2020

driusan Nov 5, 2020

driusan Nov 5, 2020

spell00 commented Nov 6, 2020

driusan left a comment

xlecours commented Nov 9, 2020

driusan commented Nov 9, 2020

[API] Added features for dicoms : PatientName and images: isPhantom #6899

[API] Added features for dicoms : PatientName and images: isPhantom #6899

Conversation

spell00 commented Aug 12, 2020 • edited Loading

Brief summary of changes

Testing instructions

driusan left a comment

Choose a reason for hiding this comment

xlecours commented Aug 26, 2020

driusan commented Aug 26, 2020

xlecours commented Aug 26, 2020

driusan commented Aug 26, 2020 • edited Loading

christinerogers commented Aug 27, 2020

spell00 commented Sep 1, 2020

christinerogers commented Oct 19, 2020

spell00 commented Oct 20, 2020

xlecours commented Oct 30, 2020

spell00 commented Oct 30, 2020

spell00 commented Nov 5, 2020

driusan Nov 5, 2020

Choose a reason for hiding this comment

driusan Nov 5, 2020

Choose a reason for hiding this comment

driusan Nov 5, 2020

Choose a reason for hiding this comment

spell00 commented Nov 6, 2020

driusan left a comment

Choose a reason for hiding this comment

xlecours commented Nov 9, 2020

driusan commented Nov 9, 2020

spell00 commented Aug 12, 2020 •

edited

Loading

driusan commented Aug 26, 2020 •

edited

Loading