-
Notifications
You must be signed in to change notification settings - Fork 3
sum loading: Stop defaulting to 1nm/px when pixel to nm scaling cannot be found. #181
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #181 +/- ##
==========================================
+ Coverage 74.62% 80.06% +5.43%
==========================================
Files 8 12 +4
Lines 607 928 +321
==========================================
+ Hits 453 743 +290
- Misses 154 185 +31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ns-rse
left a comment
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.
Sounds sensible but I wonder if we should log an error and return None for that image rather than fail?
It might mean that dictionary/list of images to be processed would then continue to be processed by TopoStats.
Another thought is whether we are likely to see similar problems with other file formats too? Should we also handle those in a similar manner?
AFMReader/spm.py
Outdated
| if px_to_real[0][0] == 0 and px_to_real[1][0] == 0: | ||
| pixel_to_nm_scaling = 1 | ||
| logger.info(f"[{filename}] : Pixel size not found in metadata, defaulting to 1nm") | ||
| raise ValueError(f"[{filename}] : Pixel to nm scaling could not be determined from metadata.") |
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 wonder if instead of raise ValueError we might consider LOGGER.error()?
My thinking is that if there is a directory with multiple images and only one fails then we should still be able to process the other images in that batch.
|
Oh interesting, just found out that the real scan size in nm, and the number of pixels (px) are accurate, so we can just also directly calculate it on the fly if it's not present. I'll look into it. |
|
Also agree, no output, with a warning rather than crashing. |
Further to #181 this commit calculates the `pixel_to_nm_scaling` from the `SPM.size` attributes if the `SPM.pxs` function returns a value of `0`. Includes test to check that this is used and calculated correctly.
feature(spm): calculate px scaling from size if not available `markdownlint-cli2` issues was addressed in #182.
Closes #180
Does as described. Don't think it has any extra effects?
It's important to crash when the file doesn't have a pixel to nanometre scaling, since it is better to not process, than to process and have wrong values for measurements. :)