BF: Allow save and load of empty trx#99
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #99 +/- ##
==========================================
+ Coverage 59.76% 60.06% +0.29%
==========================================
Files 12 12
Lines 2411 2454 +43
==========================================
+ Hits 1441 1474 +33
- Misses 970 980 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| normalized[key] = _normalize_dtype_dict(value) | ||
| elif isinstance(value, np.dtype): | ||
| # Normalize to little-endian for multi-byte types | ||
| if value.byteorder == "=" and value.itemsize > 1: |
There was a problem hiding this comment.
Not sure that I understand this. If we are on a big endian system, couldn't this still be set to "=" (native)? And this would change it?
There was a problem hiding this comment.
The goal here is to make sure we are in little endian whatever the machine (big endian system or little endian system).
for now, we do not want to stay in native.
If we are on a big endian system, couldn't this still be set to "=" (native)?
yes, but internally, we switch it to little endian
There was a problem hiding this comment.
Gotcha, looks good to me! I wonder if @frheault could give it another look, to make sure that I have not missed anything.
There was a problem hiding this comment.
Seems good, I tested using some tracking scripts with 0 seeds and it now works great for saving empty tractogram (lazy saving too) and loading it.
trx/trx_file_memmap.py
Outdated
| trx.header = header | ||
|
|
||
| # Handle empty TRX files early - no positions/offsets to load | ||
| if header["NB_STREAMLINES"] == 0 and header["NB_VERTICES"] == 0: |
There was a problem hiding this comment.
Could this be a or ? Since streamlines with 0 or 1 points are invalid anyway, maybe this could catch some corner case and force empty TRX?
There was a problem hiding this comment.
Agree, I hesitated because I believe we should handle this corner cases with better messaging.
At the end, that's fine. So code updated
|
All comments has been addressed, this PR can be merged |
3 participants
fix #91
This pull request improves the handling and testing of empty TRX files and ensures consistent dtype comparisons in tests.
TrxFile.deepcopyto only write positions and offsets files if the TRX is not empty, preventing unnecessary files in empty TRX archives (trx_file_memmap.py)._create_trx_from_pointerto immediately return an emptyTrxFileif the header indicates zero streamlines and vertices, avoiding attempts to load non-existent data (trx_file_memmap.py).This case has been encounter with Recobundles. Sometimes, the specific bundle is not in the tractogram so it should save an empty TRX instead of crashing.
Please, take a careful look @arokem and @frheault , I might miss something.