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

• In the filenibabel/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 runflake8 nibabel.

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 theTriangularMesh API fromBIAP 9, not really theCoordinateImage itself.

One question I have is if it would be possible to load the metadata from files and then construct something like anarray 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.

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 fromBIAP 9, not really the CoordinateImage itself.

Thanks for the feedback. The JMesh spec actually permits one to attachnode or triangle-associated data asProperties. The currently supported property keywords includeNormal,Color,Value,Tag, andSize. However, for simplicity, in this PR, I only exposed theTag properties as JMesh class'snodelabel andfacelabel 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 anarray 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 externalbjdata andjdata moduels), and it currently always loads the data to memory when parsing a file. When the data buffer is compressed, my understanding is thatnumpy.memmap orarrayproxy may not likely to work. But for raw/uncompressed bjdata (raw.bmsh) files, I could potentially add some code to allow the parser to returnnumpy.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 likethis 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.

@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 theFileBasedImage class that I currently derived theJMesh class from is still appropriate.

thanks

Codecov Report

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

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

@@            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

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

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