Add support for reading `mib` acquired with a given number of frame to skip per line
Description of the change
A few sentences and/or a bulleted list to describe and motivate the change:
- Added a function to read the navigation shape and check for lineskips of a .mib dataset from the corresponding .hdr file. This should help with #270.
- Fixed some typos in various docstrings.
Progress of the PR
- [x] Change implemented (can be split into several points),
- [x] update docstring (if appropriate),
- [ ] update user guide (if appropriate),
- [x] add a changelog entry in the
upcoming_changesfolder (seeupcoming_changes/README.rst), - [x] Check formatting of the changelog entry (and eventual user guide changes) in the
docs/readthedocs.org:rosettasciiobuild of this PR (link in github checks) - [ ] add tests,
- [x] ready for review.
Minimal example of the bug fix or the new feature
from rsciio.quantumdetector._api import parse_hdr_file, _get_navigation_shape_from_hdr
hdr = parse_hdr_file("SPEDSTEM_256x256_8cm_100pct.hdr")
# Your new feature...
navigation_shape = _get_navigation_shape_from_hdr(hdr)
print(navigation_shape)
Codecov Report
Attention: Patch coverage is 29.16667% with 17 lines in your changes missing coverage. Please review.
Project coverage is 86.20%. Comparing base (
bcb2057) to head (a4d40ba). Report is 3 commits behind head on main.
| Files | Patch % | Lines |
|---|---|---|
| rsciio/quantumdetector/_api.py | 29.16% | 14 Missing and 3 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #272 +/- ##
==========================================
- Coverage 86.33% 86.20% -0.13%
==========================================
Files 83 83
Lines 10668 10692 +24
Branches 2330 2337 +7
==========================================
+ Hits 9210 9217 +7
- Misses 939 953 +14
- Partials 519 522 +3
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thanks @emichr. As mentioned in https://github.com/hyperspy/rosettasciio/issues/270#issuecomment-2158637124, the logic needs to take into account interrupted acquisition.
Can you make and add small test files to cover this use case (and different number of frame to skip per line)? The test suite fails with this PR:
FAILED rsciio/tests/test_quantumdetector.py::test_interrupted_acquisition - assert (9,) == (4, 2)
FAILED rsciio/tests/test_quantumdetector.py::test_non_square[navigation_shape1] - assert (4, 2) == (8,)
FAILED rsciio/tests/test_quantumdetector.py::test_first_last_frame_8[kwargs0-navigation_shape1] - assert (8,) == (4, 2)
FAILED rsciio/tests/test_quantumdetector.py::test_first_last_frame_8[kwargs1-navigation_shape1] - assert (8,) == (4, 2)
FAILED rsciio/tests/test_quantumdetector.py::test_first_last_frame_8[kwargs2-navigation_shape1] - assert (8,) == (4, 2)
FAILED rsciio/tests/test_quantumdetector.py::test_first_last_frame_8[kwargs3-navigation_shape1] - assert (8,) == (4, 2)
FAILED rsciio/tests/test_quantumdetector.py::test_first_last_frame_8[kwargs4-navigation_shape1] - assert (8,) == (4, 2)
FAILED rsciio/tests/test_quantumdetector.py::test_first_last_frame_8[kwargs5-navigation_shape1] - assert (8,) == (4, 2)
FAILED rsciio/tests/test_quantumdetector.py::test_first_last_frame_8[kwargs6-navigation_shape1] - assert (8,) == (4, 2)
FAILED rsciio/tests/test_quantumdetector.py::test_navigation_shape_list_error - Failed: DID NOT RAISE <class 'TypeError'>
Thanks @emichr. As mentioned in #270 (comment), the logic needs to take into account interrupted acquisition.
Can you make and add small test files to cover this use case (and different number of frame to skip per line)? The test suite fails with this PR:
Sure, I'll look into it as soon as possible!
@ericpre There was a bug in my PR that caused the tests to fail. I fixed the bug and it should run fine now - sorry for not checking sooner!
I plan on making some test data soon with the updated Merlin software with various linetriggers, but it will take some time.
This sounds good. Are you okay with including the test file as part of the PR? I would like to review with the test file. When you make the test file, can you also make one with interrupted acquisition?
Would it make sense to actually skip the frame(s) (by slicing the data array in the right place?), instead of increasing the navigation shape?
This sounds good. Are you okay with including the test file as part of the PR? I would like to review with the test file. When you make the test file, can you also make one with interrupted acquisition?
I probably won't be able to make the test data files this week, so the tests will have to wait a bit. I'm okay with waiting for those and including them in the PR when ready.
Would it make sense to actually skip the frame(s) (by slicing the data array in the right place?), instead of increasing the navigation shape?
Yes, that's a good idea. It required very little changes, and I think I found the right place to do it. Let me know if it's not the best way of doing it.
Hello! Just want to say I am looking into a related issue with some .mib files.
The datasets use an ROI and should therefore be about (512, 512, 64, 256) in shape. Using the current dev version gives an error. However, using this fix I get the following shape: (510, 514, 256, 64)
So it seems it seems it skips lines in the wrong dimensions. Additionally the probe looks like this:
I can, however, fix my dataset by loading it like this, using the current dev version of rosettasciio:
s = hs.load(file, navigation_shape=(514,512))
s = s.inav[:512,:]
Thus manually trimming the lineskip! @emichr let me know if you want to look into this example further.
s = hs.load(file, navigation_shape=(514,512)) s = s.inav[:512,:]Thus manually trimming the lineskip! @emichr let me know if you want to look into this example further.
Hi @sivborg, great that you tested this! Perhaps it is better to just modify the docstring with this as an example (and possibly making a note in the documentation as well) instead of including my suggested code changes? What do you think @ericpre?
A simple approach would be to specify the number of frames to skip, as mentioned in comment in https://github.com/hyperspy/rosettasciio/issues/270#issuecomment-2158650368. This would be more convenient for the user, rather having to remember/figure out where the add the additional frames.
I don't remember the details, but I would expect that it could be implemented without loading the frames to skip, which would be more efficient, particularly when loading lazily.
@emichr, have you been able to make the test files required to finish this pull request?
@emichr, @sivborg: gentle ping on this PR, a small test file is needed. :)