AFM-SPM / TopoStats

An AFM image analysis program to batch process data and obtain statistics from images
https://afm-spm.github.io/TopoStats/
GNU Lesser General Public License v3.0
55 stars 10 forks source link

Add the option to save as a `tif` file #778

Closed derollins closed 4 months ago

derollins commented 5 months ago

I noticed while testing PR #777 that tif was not available as an option for saving images. Although this may be due to inability of matplotlib to embed metadata into these files (https://matplotlib.org/stable/api/_as_gen/matplotlib.pyplot.savefig.html), tif files are a standard for lot of image analysis programmes that users may use in conjunction with Topostats as well as being the preferred format for many journal.

For these reasons I thought I would add tif to validation.py and the deafult_congif.yaml file and update documentation to reflect this update.

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (8bfa700) 84.36% compared to head (9f5d98d) 84.46%. Report is 13 commits behind head on main.

:exclamation: Current head 9f5d98d differs from pull request most recent head 50f94db. Consider uploading reports for the commit 50f94db to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #778 +/- ## ========================================== + Coverage 84.36% 84.46% +0.09% ========================================== Files 21 21 Lines 3134 3134 ========================================== + Hits 2644 2647 +3 + Misses 490 487 -3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ns-rse commented 5 months ago

Thanks for this PR @derollins .

I looks like you've used the ns-rse/776-config-jigging branch as a basis for your derollins/tiff_validation which means this Pull Request includes all of the changes that are out for review in #777.

Normally, although not exclusively, new branches should be made from main (and after git pull which ensures the local copy is up-to-date.

We can work round this though, perhaps most simply by waiting for #777 to be more widely tested, reviewed and merged first.

One question I would have, which might influence the implementation of supporting the TIFF format is whether the loss of metadata is likely to be problematic further down the line?

There are other ways of writing such files some of which may include metadata.

A quick scan also suggests that TIFFs can hold multiple images, is this something that is likely to be required for example when working with .asd images which contain multiple images that essentially constitute a video?

derollins commented 5 months ago

Thanks for this PR @derollins .

I looks like you've used the ns-rse/776-config-jigging branch as a basis for your derollins/tiff_validation which means this Pull Request includes all of the changes that are out for review in #777.

Normally, although not exclusively, new branches should be made from main (and after git pull which ensures the local copy is up-to-date.

We can work round this though, perhaps most simply by waiting for #777 to be more widely tested, reviewed and merged first.

One question I would have, which might influence the implementation of supporting the TIFF format is whether the loss of metadata is likely to be problematic further down the line?

There are other ways of writing such files some of which may include metadata.

A quick scan also suggests that TIFFs can hold multiple images, is this something that is likely to be required for example when working with .asd images which contain multiple images that essentially constitute a video?

Thanks for the feedback @ns-rse. I assumed the #777 PR would probably be merged before this so worked off that but I'll go back to main in the future.

I have never looked at the image output metadata, but having quickly opened it, doesn't look like there is much there at the moment and both the .png files and .tiff files generated by topostats have only dimensions in their metadata so I don't think this will be an issue. If this does become an issue in the future it looks like it should be possible to save TIFF files with metadata by using the PIL library but don't think it will be worth adding this now.

There is a lot of advantages to the TIFF image format, when I first started using 'old' topostats it was the default image format I used (generated though the gwydion module) until I swapped to .png because you can't use .tiff images in google docs. The ability to contain multiple images is certainly an something that could be useful in SPM image processing as although high speed .asd files might be better processed as videos (although it could be saved as a TIFF too I expect) most SPM 'images' contain multiple channel (i.e. height, phase, adhesion, charge etc.) which could potentially be conveniently combined.

I'm no optical microscopy expert but I know TIFF 'stacks' of many images either in a time or depth series are generated by other microscopy techniques and can be opened and processed in software such as ImageJ.

ns-rse commented 4 months ago

I think we should ultimately be look at using PIL to save metadata to the images as its fairly key information.

I'll try and get round to looking at this in the coming weeks.

In the meantime there is a tpyo that is causing the pre-commit checks to fail.

MaxGamill-Sheffield commented 4 months ago

@ns-rse @derollins I think with Eddie's changes this should be marked as completed and pushed asap as we can now save the files as tiff and tif which are essential for publications.

The need for metadata in the Tiff files isn't a big priority and I think it should be a new issue.