feature: new reader for .h5-jpk files#152
Conversation
Derollins/verticallyflipped jpk
ns-rse
left a comment
ns-rse
left a comment
There was a problem hiding this comment.
Apologies @derollins I started reviewing this the other week and forgot to finish the job and only just realised.
A few minor comments in-line but looking good.
Appreciate you taking the time to contribute these improvements.
| elif self.suffix == ".spm": | ||
| image, pixel_to_nanometre_scaling_factor = spm.load_spm(self.filepath, self.channel) | ||
| elif self.suffix == ".h5-jpk": | ||
| image, pixel_to_nanometre_scaling_factor, _ = h5_jpk.load_h5jpk(self.filepath, self.channel) |
There was a problem hiding this comment.
Will people never want the timestamps that are returned?
Or is it the case that for the general_loader they won't be required?
There was a problem hiding this comment.
I'm following the format of the .asd loader here. Since I don't know much about the application of the general loader (outside the reference to napari in the docstring) I thought I would just follow the lead of the existing code.
Both .asd and .h5-jpk are 'high-speed' formats and the temporal information is very important for this sort of data, however I don't know if it is required in the context that the general loader is designed.
P.S. I have a little project dealing with this sort of data within a time-aware framework: playNano
There was a problem hiding this comment.
I think the general_loader is @MaxGamill-Sheffield work and for now it should consistently just return image and pixel_to_nanometre_scaling_factor so don't worry about this.
playNano looks great, I hadn't realised you were so into your coding. It looks in really good shape from the FAIR for Research Software perspective too with good documentation and I like that you're using pre-commit and linting everything 👍 . If you wanted to have it reviewed in some manner there is a useful tool from Scientific Python Repo Review
There was a problem hiding this comment.
Thanks, it's still quite new and very much a work in progress, but I'm building it with FAIR in mind. Right now, I'm focused on laying down the final pieces of the foundation. Once that's in place, I plan to do a FAIR review and plan before introducing more features. Many of the linting and testing tools were inspired by my experience working with you and the TopoStats team!
There was a problem hiding this comment.
🆒 , you could consider publishing it in the Journal of Open Source Software when you're happy with it.
|
@ns-rse Should I go through the tests for AFMReader and add |
There was a problem hiding this comment.
Thanks @derollins
Tests are failing under Python 3.11/3.12 because of precision issues which I've fixed in #155. I would suggest updating your branch and which should update this PR but am wary of possible merge conflicts so am happy to merge this.
The one thing I forgot when reviewing was updating docs/usage.md to include .h5-jpk in the supported file formats but I'm adding to that at the moment myself and will include it so nothing to worry about.
|
Great, thanks Neil! |
2 participants
Bruker/ JPK have started using a new file type for high speed systems (incl. Nanoracer) rather than images being saves individually frames from a high speed 'video' are saved together in one HDF5 file.
This reader follows the format of the other readers within the library, adding two extra parameters:
flip_image, also used in the updated.jpkreader, it appears that the new.h5-jpkimages also need to be flipped,frame, an integer to indicates which frame should be accessed.The output is then the same as for the other loaders.
This PR includes:
h5_jpk.pytest_h5jpk.py(withsample_0.h5_jpkadded totests/resourcesTests for the file loading, reading and another added for when the frame number provided is out of range.