Add support for reading mib acquired with a given number of frame to skip per line

[emichr]

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 Shape of 4DSTEM .mib data triggered with lineskip=1 #270.
• Fixed some typos in various docstrings.

Progress of the PR

• Change implemented (can be split into several points),
• update docstring (if appropriate),
• update user guide (if appropriate),
• add a changelog entry in the upcoming_changes folder (see upcoming_changes/README.rst),
• Check formatting of the changelog entry (and eventual user guide changes) in the docs/readthedocs.org:rosettasciio build of this PR (link in github checks)
• add tests,
• 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)

SPEDSTEM_256x256_8cm_100pct.txt

[codecov]

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 ⚠️

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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ericpre]

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:

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

[emichr]

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!

[emichr]

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

[ericpre]

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?

[emichr]

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.

[sivborg]

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.

[emichr]

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?

[ericpre]

A simple approach would be to specify the number of frames to skip, as mentioned in comment in #270 (comment). 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.