Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 57 additions & 1 deletion gufe/visualization/mapping_visualization.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# This code is part of OpenFE and is licensed under the MIT license.
# For details, see https://github.com/OpenFreeEnergy/gufe
from typing import Any, Collection, Optional
from typing import Any, Collection, Optional, List
from itertools import chain
from PIL import Image, ImageOps
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe pillow now replaces PIL, which should be used instead.

Additionally - we would need to work out what we want to do with this additional dependency. Probably would need to be an optional.

Copy link
Copy Markdown
Contributor

@jthorton jthorton Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use pillow I think rdkit has that as part of its recipe.

from io import BytesIO

from rdkit import Chem
from rdkit.Chem import Draw
Expand Down Expand Up @@ -305,3 +307,57 @@ def draw_unhighlighted_molecule(mol, d2d=None):
bond_colors=[{}],
highlight_color=red,
)


def _concat_images(imgs, buffer_spacing=4, ncols=3)-> Image:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the function that was used to create the grid in the example? This should probably be used by the draw_multipule_mappings function to create a grid layout.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, this PR is a very old one and I wanted to bring it to your attention as I think it could be a handy feature.

hm... I wonder if this function should be used in line 360 somehow? I also wonder if the code developed away from the initial implementation?

"""
This helper function is constructing a Grid Image of given imgs.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if it's private, this really could use with a better explanation of what the method does.

"""
height = 0
width = 0

#Calculate number of rows
nrows= (len(imgs)+1)//ncols if(len(imgs)%3>0) else len(imgs)//ncols

#Get total widht and height of img
for img in imgs:
height = max(height, img.height)
width = max(width, img.width)

height = height * nrows
width = width * ncols

width += buffer_spacing*ncols
height += buffer_spacing*nrows

#Construct Concatentated Image
res = Image.new("RGBA",(width,height))

x = 0
y = 0
for i, img in enumerate(imgs):
img = ImageOps.expand(img, border=3, fill="grey")
res.paste(img,(x,y))
x += img.width + buffer_spacing

if(i%ncols == 0 and i!=0):
y+= img.height + buffer_spacing
x=0

return res

def draw_multiple_mappings(mappings:List, out_file_path:str=None)->Image:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the type hints, for mappings and out_file_path, maybe change the second to be output_file. We should expose the grid layout settings as well to recreate the example.

"""
This function generates an image, consisting of multiple mapping visualization.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a public facing method, it would need better docstring, especially on the "what does it do and what are my parameters".

"""
views = []
for m in mappings:
view = draw_mapping(m._compA_to_compB, m.componentA.to_rdkit(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would really need tests of some kind, even if they are smoke tests (although I do note that matplotlib has a series of methods to test image equality that could be useful here).

m.componentB.to_rdkit(),)
bio = BytesIO(view)
view = Image.open(bio)
views.append(view)
res = show_images(views)

if(out_file_path is not None):
res.save(out_name)