-
Notifications
You must be signed in to change notification settings - Fork 35
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added four fiducial points. #125
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments in review and on Landmarks Google Doc
https://docs.google.com/document/d/1Z7Uk26AF4FuG01MWFkV3dUWN-8w_oPMkmdpKmgohiuw/edit#heading=h.6v65w6lpo73
if includeUreter: | ||
# Left ureter | ||
nodeIdentifier += 1 | ||
idx1 = elementsCountAlong * elementsCountAround + elementsCountAroundUreter | ||
element1 = mesh.findElementByIdentifier(idx1) | ||
markerUreter1Point = markerPoints.createNode(nodeIdentifier, markerTemplateInternal) | ||
cache.setNode(markerUreter1Point) | ||
markerName.assignString(cache, 'ureter junction of bladder trigone') | ||
markerLocation.assignMeshLocation(cache, element1, [0.0, 1.0, 1.0]) | ||
# Right ureter | ||
nodeIdentifier += 1 | ||
idx2 = elementsCountAlong * elementsCountAround + 2 * elementsCountAroundUreter | ||
element2 = mesh.findElementByIdentifier(idx2) | ||
markerUreter2Point = markerPoints.createNode(nodeIdentifier, markerTemplateInternal) | ||
cache.setNode(markerUreter2Point) | ||
markerName.assignString(cache, 'ureter junction of bladder trigone') | ||
markerLocation.assignMeshLocation(cache, element2, [0.0, 1.0, 1.0]) | ||
else: | ||
# Left ureter | ||
nodeIdentifier += 1 | ||
idx1 = ureterElementPositionDown * elementsCountAround + ureterElementPositionAround + 1 | ||
element1 = mesh.findElementByIdentifier(idx1) | ||
markerUreter1Point = markerPoints.createNode(nodeIdentifier, markerTemplateInternal) | ||
cache.setNode(markerUreter1Point) | ||
markerName.assignString(cache, 'ureter junction of bladder trigone') | ||
markerLocation.assignMeshLocation(cache, element1, [ureter1Position.xi1, ureter1Position.xi2, 1.0]) | ||
# Right ureter | ||
nodeIdentifier += 1 | ||
idx2 = ureterElementPositionDown * elementsCountAround + elementsCountAround - ureterElementPositionAround | ||
element2 = mesh.findElementByIdentifier(idx2) | ||
markerUreter2Point = markerPoints.createNode(nodeIdentifier, markerTemplateInternal) | ||
cache.setNode(markerUreter2Point) | ||
markerName.assignString(cache, 'ureter junction of bladder trigone') | ||
markerLocation.assignMeshLocation(cache, element2, [1-ureter1Position.xi1, ureter1Position.xi2, 1.0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like having 4 sets of code to make equivalent points. Prone to maintenance problems.
Can you propose a way to store the points in a structure, e.g. a dict { "name" : "ureter...", "element_id" : idx1, "xi" : [ x, y, z ] }. Then make the 2 points in common code below. I like to see minimum code in special case clauses.
Need to qualify names with left/right.
Do you mean 'ureter junction with bladder trigone'? Is it necessary to specify trigone?
marker_dic = {"name": ["Apex of urinary bladder", " left ureter junction with bladder", "right ureter junction with bladder", | ||
"urethra junction with bladder dorsal", "urethra junction with bladder ventral"], "element_id": element_id, "xi": xi} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better, but I suggest you rearrange all the above code to get the names at the top and assign them to variables e.g.
apexMarkerName = "apex of urinary bladder"
leftUreterMarkerName = "left ureter junction with bladder"
rightUreterMarkerName = "right ureter junction with bladder"
dorsalUrethraMarkerName = "urethra junction with bladder dorsal"
ventralUrethraMarkerName = "urethra junction with bladder ventral"
Then you can go through all your code cases and add markers to the following list:
markers = []
markers.append({ "name" : apexMarkerName, "element" : element, "xi" : xi })
if includeUreter:
...
markers.append({ "name" : leftUreterMarkerName , "element" : element, "xi" : xi })
...
else:
...
At the end you can do a for loop over the markers and add them all.
The benefit is that you have the text names written once, the language will catch you if you put an incorrect variable name in for the marker, and each record of a marker point keeps name, element and xi together. All helpful for maintenance.
Also note that you had some capitals and leading whitespace in the names in your array...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good! In your next commit, can you please add some testing for the annotation groups as you can see in the heart, lung and stomach tests.
No description provided.