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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge(?): World coordinate system values #123

Merged
merged 4 commits into from
Aug 19, 2014

Conversation

cmaumet
Copy link
Member

@cmaumet cmaumet commented Jul 17, 2014

Following discussion at #52 with @nicholst, @satra and @jbpoline, a first implementation of the WorldCoordinateSystem terms is included in this pull request.

The current hierarchy is as follows:
image

Definitions must be re-worked. In particular, for each CoordinateSystem, we would need to specify (as suggested by @satra):

  1. a set of images
  2. a normalization algorithm
  3. the previous template

@nicholst
Copy link
Contributor

+1

@cmaumet cmaumet changed the title WIP: Coordinate space values WIP: Coordinate system values Jul 30, 2014
@cmaumet cmaumet changed the title WIP: Coordinate system values WIP: World coordinate system values Jul 30, 2014
@cmaumet
Copy link
Member Author

cmaumet commented Jul 31, 2014

This PR has just been rebased.

Do you think we could merge these changes?

@cmaumet cmaumet changed the title WIP: World coordinate system values Merge(?): World coordinate system values Jul 31, 2014
@cmaumet cmaumet added the merge label Jul 31, 2014
@satra
Copy link
Contributor

satra commented Jul 31, 2014

@cmaumet - this looks good.

my two worries (not necessary to prevent merging for now) are:

  1. can we keep URLs as short as possible? as such could create further hierarchies for the MNI152Nonlinear[a|b|c][Asymmetric|Symmetric]CoordinateSystem and thereby reduce the term name.
  2. i think "CustomCoordinateSystem" should be parallel to worldCS. not that it matters in brain imaging now, but you could fix a coordinate system in space, what if they put a portable scanner on the international space station or as part of the one way trip to mars in 2024 :)

btw @khelm do scanners contain information about orientation of the bore and latitude/longitude of the scanner.

@cmaumet
Copy link
Member Author

cmaumet commented Jul 31, 2014

@satra: thank you for your comments. Regarding your point 2: I am wondering if World is the appropriate prefix...

I liked World because it goes well with the voxelToWorldMapping attribute. In this thread on the nipy mailing list (pointed out earlier by @jbpoline) they used both "world coordinate"/"world space" and "reference space".

Would Reference be clearer than World?

@khelm
Copy link
Contributor

khelm commented Jul 31, 2014

@satra: I will check. I know it's been difficult to get from vendors info on their gradient coordinate system, how they define the patient orientation etc, so I don't know if they put it in DICOM or not. Lat/Long would be a surprise.

@nicholst
Copy link
Contributor

nicholst commented Aug 2, 2014

+1 to merge

@cmaumet
Copy link
Member Author

cmaumet commented Aug 19, 2014

Thank you for the comments! I will now merge those changes. I have created issue #139 to continue the discussion on the naming and hierarchy of those terms.

cmaumet pushed a commit that referenced this pull request Aug 19, 2014
Merge(?): World coordinate system values
@cmaumet cmaumet merged commit 6d2b938 into incf-nidash:master Aug 19, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants