ENH: Reading and writing JSON/binary JSON based surface data (.jmsh and .bmsh)

[fangq]

hi @effigies, I am following up on the discussions from #1065, and would like to use this thread to propose a patch to support .jmsh/.bmsh based surface mesh data formats.

To recap on my previous presentation during the surface mesh format meeting:

• .jmsh is basically a plain JSON file using JMesh annotations to store surface mesh data structures (nodes as MeshVertex3, triangles as MeshTri3)
• .bmsh is the same as .jmsh except that the underlying serialization uses a binary JSON format - BJData
• via the JData specification (data-structure level annotations), both formats support data-record-level compression (zlib, gzip, 'lzma' and potentially others)

thanks to @neurolabusc's mesh format benchmarks, the loading speed and file sizes of these files are summarized/compared in this post.

Here, I would like to show you a lightweight patch (draft) to allow nibabel to read/write .jmsh and .bmsh files. A screenshot showing the interface can be found below.

This patch requires two external lightweight packages as dependencies, both can be installed via pip:

• jdata (14 kB) - handling JData annotation encoding/decoding and file IO and
• bjdata (63kB) - handling bjdata reading/writing

because I am not entirely familiar with nibabel's IO classes, currently, I based the JMesh class over FileBasedImage. I understand that CoordinateImage is currently being developed in #1090, and perhaps it is the better root class for adding new mesh formats.

Let me know if you have any comments about this draft. I would be happy to add some tests and open to suggestions to continue polishing this patch.

[pep8speaks]

Hello @fangq, thank you for submitting the Pull Request!

• In the file nibabel/jmesh/jmesh.py:

Line 13:19: E231 missing whitespace after ','
Line 13:26: E231 missing whitespace after ','
Line 13:34: E231 missing whitespace after ','
Line 20:19: E231 missing whitespace after ':'
Line 21:14: E231 missing whitespace after ':'
Line 22:23: E231 missing whitespace after ':'
Line 23:13: E231 missing whitespace after ':'
Line 24:17: E231 missing whitespace after ':'
Line 28:17: E231 missing whitespace after ':'
Line 32:21: E231 missing whitespace after ':'
Line 33:14: E231 missing whitespace after ':'
Line 34:12: E231 missing whitespace after ':'
Line 38:1: E302 expected 2 blank lines, found 1
Line 74:12: E714 test for object identity should be 'is not'
Line 77:12: E714 test for object identity should be 'is not'
Line 78:86: E202 whitespace before '}'
Line 85:12: E714 test for object identity should be 'is not'
Line 86:83: E202 whitespace before '}'
Line 102:1: E302 expected 2 blank lines, found 1
Line 119:29: E231 missing whitespace after ','
Line 124:5: E265 block comment should start with '# '
Line 126:5: E265 block comment should start with '# '
Line 130:5: E265 block comment should start with '# '
Line 132:5: E265 block comment should start with '# '
Line 146:37: E231 missing whitespace after ','
Line 149:5: E265 block comment should start with '# '
Line 151:5: E265 block comment should start with '# '
Line 163:37: E231 missing whitespace after ','
Line 166:5: E265 block comment should start with '# '
Line 168:5: E265 block comment should start with '# '
Line 170:39: E714 test for object identity should be 'is not'
Line 170:101: E501 line too long (105 > 100 characters)
Line 173:39: E714 test for object identity should be 'is not'
Line 173:101: E501 line too long (105 > 100 characters)
Line 176:44: E714 test for object identity should be 'is not'
Line 176:101: E501 line too long (120 > 100 characters)
Line 179:44: E714 test for object identity should be 'is not'
Line 179:101: E501 line too long (120 > 100 characters)
Line 184:1: E302 expected 2 blank lines, found 1
Line 209:34: E714 test for object identity should be 'is not'
Line 210:31: E225 missing whitespace around operator
Line 211:34: E714 test for object identity should be 'is not'
Line 212:43: E714 test for object identity should be 'is not'
Line 213:36: E225 missing whitespace around operator
Line 215:36: E225 missing whitespace around operator
Line 217:34: E714 test for object identity should be 'is not'
Line 218:38: E272 multiple spaces before keyword
Line 218:44: E714 test for object identity should be 'is not'
Line 219:33: E225 missing whitespace around operator
Line 221:33: E225 missing whitespace around operator
Line 225:1: E305 expected 2 blank lines after class or function definition, found 1

To test for issues locally, pip install flake8 and then run flake8 nibabel.

[effigies]

Thanks @fangq! This is great to see. If I'm reading this right, jmsh/bmsh are specifically surface meshes, not data sampled to surface meshes? If so, then it corresponds to the TriangularMesh API from BIAP 9, not really the CoordinateImage itself.

One question I have is if it would be possible to load the metadata from files and then construct something like an array proxy so that the data arrays do not need to be loaded until called for? If I recall, your files are not actually gzipped, only the data arrays, so we can't use our existing file handlers and proxies as drop-in loaders, but I suspect we could figure something out.

[fangq]

Thanks @fangq! This is great to see. If I'm reading this right, jmsh/bmsh are specifically surface meshes, not data sampled to surface meshes? If so, then it corresponds to the TriangularMesh API from BIAP 9, not really the CoordinateImage itself.

Thanks for the feedback. The JMesh spec actually permits one to attach node or triangle-associated data as Properties. The currently supported property keywords include Normal, Color, Value, Tag, and Size. However, for simplicity, in this PR, I only exposed the Tag properties as JMesh class's nodelabel and facelabel properties - others can also be easily exposed if see a need.

In this case, should I derive JMesh from TriangularMesh class?

One question I have is if it would be possible to load the metadata from files and then construct something like an array proxy so that the data arrays do not need to be loaded until called for?

Loading array buffers from the file is handled in the parser level (i.e. the external bjdata and jdata moduels), and it currently always loads the data to memory when parsing a file. When the data buffer is compressed, my understanding is that numpy.memmap or arrayproxy may not likely to work. But for raw/uncompressed bjdata (raw.bmsh) files, I could potentially add some code to allow the parser to return numpy.memmap objects if user prefers. For now, let's assume this memmap based access is not yet supported.

By the way, do you have a code formatter (something like this for C++) so that I can get rid of CI warnings?

also, what command to run all tests in nibabel? I just want to follow the examples of other modules and start building some tests.

[fangq]

@effigies, sorry for dropping the ball. I would like to spend some time to get this PR polished.

I see you had added a formatter (blue) in #1124, I would like to know what is the command I should run to apply the style to format my code.

Also, per your earlier comment, jmesh can associate values to vertices/triangles - similar to Gifti, please let me know if the FileBasedImage class that I currently derived the JMesh class from is still appropriate.

thanks

[codecov]

Codecov Report

Patch coverage: 24.46% and project coverage change: -0.54 ⚠️

Comparison is base (cbd7690) 92.16% compared to head (2c8942f) 91.63%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1114      +/-   ##
==========================================
- Coverage   92.16%   91.63%   -0.54%     
==========================================
  Files          97       99       +2     
  Lines       12332    12428      +96     
  Branches     2534     2563      +29     
==========================================
+ Hits        11366    11388      +22     
- Misses        645      718      +73     
- Partials      321      322       +1     

Impacted Files	Coverage Δ
nibabel/jmesh/jmesh.py	21.11% <21.11%> (ø)
nibabel/__init__.py	100.00% <100.00%> (ø)
nibabel/imageclasses.py	100.00% <100.00%> (ø)
nibabel/jmesh/__init__.py	100.00% <100.00%> (ø)

... and 7 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[effigies]

Thanks @fangq. I'll try to have a look at this in more detail in the coming week.