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 Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 79.93%. Comparing base (ac9753c) to head (4902de5).
⚠️ Report is 232 commits behind head on main.

Files with missing lines Patch % Lines
AFMReader/spm.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #181      +/-   ##
==========================================
+ Coverage   74.62%   79.93%   +5.30%     
==========================================
  Files           8       12       +4     
  Lines         607      927     +320     
==========================================
+ Hits          453      741     +288     
- Misses        154      186      +32     

☔ 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?

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.

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