-
Notifications
You must be signed in to change notification settings - Fork 3
feature: new reader for .h5-jpk files
#152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
41 commits
Select commit
Hold shift + click to select a range
4e776e5
Merge pull request #1 from derollins/derollins/verticallyflipped_jpk
derollins e1709d9
Merge branch 'AFM-SPM:main' into main
derollins a4b5a15
Merge branch 'AFM-SPM:main' into main
derollins ccb904a
Add reader for .h5-jpk files
derollins 8125e01
get pre-commit working
derollins 6867323
[pre-commit.ci] Fixing issues with pre-commit
pre-commit-ci[bot] 77f3a62
Fix pre commit issues
derollins d48f660
fix pre commit issues
derollins 9e678a9
Merge branch 'derollins-h5jpk_precommit' into derollins/h5jpk
derollins fb0456a
[pre-commit.ci] Fixing issues with pre-commit
pre-commit-ci[bot] e55e9e5
modularise load-h5jpk funciton
derollins d1f59d2
conflicts resolved
derollins 3cf6b8c
[pre-commit.ci] Fixing issues with pre-commit
pre-commit-ci[bot] 02cd920
precommit remove double line break
derollins da11b5c
Merge branch 'derollins/h5jpk' of https://github.com/derollins/AFMRea…
derollins 5a24577
updated readme and general loader
derollins 8755b82
[pre-commit.ci] Fixing issues with pre-commit
pre-commit-ci[bot] e3d32af
Update general_loader with h5-jpk
derollins f930a71
add tests for .hp-jpk file reader
derollins 3b96bf3
add tests for .hp-jpk file reader
derollins fa28960
[pre-commit.ci] Fixing issues with pre-commit
pre-commit-ci[bot] cfd3962
fix tests
derollins 58d2a06
merge pre-commit fixes
derollins 4915e34
fix tests flip_image
derollins 2eb5e80
[pre-commit.ci] Fixing issues with pre-commit
pre-commit-ci[bot] 5d40c32
fix tests float type
derollins 3e2745e
merge precommit lint
derollins 0dbff27
[pre-commit.ci] Fixing issues with pre-commit
pre-commit-ci[bot] 8a31006
add h5-jpk test docstring
derollins 0bff7a6
fix docstring in test_h5jpk
derollins 6ef117c
[pre-commit.ci] Fixing issues with pre-commit
pre-commit-ci[bot] 6dce2f6
added test for frame out of range
derollins 1aa8960
load .h5-jpk files as 3D numpy array 'frames' and output timestamps
derollins 726591f
[pre-commit.ci] Fixing issues with pre-commit
pre-commit-ci[bot] be310a5
fix load_h5jpk docstring
derollins 7c22382
Merge branch 'derollins/h5jpk' of https://github.com/derollins/AFMRea…
derollins 6c8f4cf
[pre-commit.ci] Fixing issues with pre-commit
pre-commit-ci[bot] 0450b43
update general loader, Readme and examples
derollins ae121e2
Merge branch 'derollins/h5jpk' of https://github.com/derollins/AFMRea…
derollins 807d5d6
Respond to review from Neil
derollins 314114d
Update _get_channel_info docstring
derollins File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will people never want the
timestampsthat are returned?Or is it the case that for the
general_loaderthey won't be required?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm following the format of the
.asdloader 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
.asdand.h5-jpkare '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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the
general_loaderis @MaxGamill-Sheffield work and for now it should consistently just returnimageandpixel_to_nanometre_scaling_factorso 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-commitand linting everything 👍 . If you wanted to have it reviewed in some manner there is a useful tool from Scientific Python Repo ReviewThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🆒 , you could consider publishing it in the Journal of Open Source Software when you're happy with it.