Skip to content

Conversation

@SylviaWhittle
Copy link
Collaborator

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. :)

@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.06%. Comparing base (ac9753c) to head (2045727).
⚠️ Report is 238 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@ns-rse ns-rse left a 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.")
Copy link
Collaborator

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.

@SylviaWhittle
Copy link
Collaborator Author

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.

@SylviaWhittle
Copy link
Collaborator Author

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.
@ns-rse ns-rse merged commit f6fc37d into main Jan 26, 2026
12 checks passed
@ns-rse ns-rse deleted the SylviaWhittle/stop-default-1nm-p2nm branch January 26, 2026 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feature] : Crash instead of default to 1.0 for pixel to nm scaling for .spm images.

4 participants