-
Notifications
You must be signed in to change notification settings - Fork 0
Create CLEM PNG thumbnails for PATo #221
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
…colourised PNG thumbnails for PATo visualisation
… and height tuples different from NumPY arrays
…offline installers in
…er for a specified version before attempting to download it
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #221 +/- ##
==========================================
+ Coverage 82.71% 82.98% +0.27%
==========================================
Files 62 62
Lines 8112 8342 +230
Branches 1130 1218 +88
==========================================
+ Hits 6710 6923 +213
- Misses 960 974 +14
- Partials 442 445 +3 🚀 New features to boost your workflow:
|
… its supported scope
stephen-riggs
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.
Looks fine. A couple of minor comments.
Also in the Dockerfiles there are currently various references to a "packages" folder which will can probably become your "installers" folder, but maybe not in this PR
| # Collect image frames | ||
| frames: list[PIL.Image.ImageFile.ImageFile] = [] | ||
| try: | ||
| while True: |
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.
Should be possible to read using tifffile and loop over the frames rather than doing the image seek. Not essential, just my preference for for loops rather than while blocks
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'll have a look at replacing it with the use of tifffile, sure. I just need to make sure that the image doesn't get flipped upside down when doing so, as I'm aware that different image/plotting packages treat the y-axis differently.
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.
Found a solution involving minimal changes to the existing logic. Let me know what you think!
| "composite_image": str(save_name.resolve()), | ||
| "output_file": str(save_name), | ||
| "thumbnail": str(save_name.parent / ".thumbnails" / f"{save_name.stem}.png"), | ||
| "thumbnail_size": (512, 512), # height, row |
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.
Is there a reason for choosing to hard-code the 512, 512 here rather than in the images service? I think all of the other plugins do it in the images service
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.
The thumbnail size is needed on the Murfey side to work out the relative positions of the grid square thumbnails on the atlas thumbnail. At the point the message is passed back to Murfey, you cannot be sure that the 'images' plugin will have generated the thumbnail, so trying to read the dimensions by opening the thumbnail will potentially fail.
As discussed, the repetition could be avoid by setting the target thumbnail size on Murfey when the image processing request is first sent to CryoEM Services. I can implement this in a subsequent PR further down the line.
…ead of continuously seeking frames in a 'while True: ...' block
stephen-riggs
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.
The loop looks much nicer now, thanks for changing it
TIFFs apparently cannot be used for web-based data visualisation, so animated PNGs of the processed images/image stacks need to be created.
This PR adds logic to the CLEM workflows (both the services and wrappers) to send additional messages to the Images service to create scaled down animated PNGs of the processed images. These are saved in hidden
.thumbnailsfolders in their parent image's directory.Additionally, the
cryoemservices_cpuDockerfile was updated to accommodate the presence of offline IMOD installers, instead of forcibly downloading from the IMOD website every time. A.dockerignorefile was also added to the repository to ensure only the bare minimum files are copied across to the Docker image during the build. The.gitignorefile was also updated to ignore the cache directories of the newer pre-commit hooks.