[BUG, MRG] Fix ACPC Coordinates in surface RAS when they should be in scanner RAS

[alexrockhill]

Fixes #989.

It's a bit inelegant but I encapsulated the ugly parts behind helper functions. MNE uses m and surface RAS and nibabel/BIDS uses mm (generally) and scanner RAS which makes a lot of headaches. I think this is about as clean as possible but let me know if there are things that should be fixed. This is a pretty high priority also because main is wrong will cause incorrectly formatted datasets.

[hoechenberger]

I'm wondering if this shouldn't better be implemented as methods of DigMontage upstream in MNE? What am I missing?

[alexrockhill]

I'm wondering if this shouldn't better be implemented as methods of DigMontage upstream in MNE? What am I missing?

Yes, that would be nice if they were but then we'd have to require the development version of MNE. I can do a PR now to add to MNE-Python and then we can change it next MNE-Python release.

EDIT: I looked into it and montage already has apply_trans and I don't think there is really a convenience function that fits. We could add transform_to_ras like transform_to_head but I'm not sure that belongs in MNE-Python.

[hoechenberger]

@larsoner If you have a few minutes, could you please share your thoughts on this one?

[codecov]

Codecov Report

Merging #990 (3f6c46d) into main (71130fc) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #990      +/-   ##
==========================================
+ Coverage   95.07%   95.11%   +0.04%     
==========================================
  Files          25       25              
  Lines        3713     3744      +31     
==========================================
+ Hits         3530     3561      +31     
  Misses        183      183              

Impacted Files	Coverage Δ
mne_bids/config.py	97.59% <ø> (ø)
mne_bids/__init__.py	100.00% <100.00%> (ø)
mne_bids/dig.py	98.60% <100.00%> (+0.16%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71130fc...3f6c46d. Read the comment docs.

[alexrockhill]

LGTM?

[larsoner]

I would also 1) avoid the scale stuff by being careful about units, instead, and either way 2) use combine_transforms and apply once. But the given approach seems okay as is so +1 for merge after fixing the test collection bloat issue

[larsoner]

Yes to avoid the scale you have to either:

1. Divide the first three rows by 1000, e.g., for a vox2ras, which has units mm/vox for the first three rows (except the last col, which has units mm) and you need to get to m/vox (or m for the last col), so you multiply by 1m/1000mm (i.e., divide by 1000), or
2. Multiply the first three columns by 1000, e.g., for a ras2vox, which has units vox/mm for the first three cols (and plain vox for the last col, which we leave alone) and you need to get to vox/m, so you multiply by 1000mm/1m (i.e., multiply by 1000)

At least I think this is the case, feel free to try it...

[alexrockhill]

Looks like it worked pretty much like you said.

[larsoner]

LGTM +1 for merge

[hoechenberger]

Thanks @alexrockhill!