-
Notifications
You must be signed in to change notification settings - Fork 9
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
Adding multiMapping plots #237
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
from io import BytesIO | ||
|
||
from rdkit import Chem | ||
from rdkit.Chem import Draw | ||
|
@@ -305,3 +307,57 @@ | |
bond_colors=[{}], | ||
highlight_color=red, | ||
) | ||
|
||
|
||
def _concat_images(imgs, buffer_spacing=4, ncols=3)-> Image: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
""" | ||
This function generates an image, consisting of multiple mapping visualization. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
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 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.
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.
If we use pillow I think rdkit has that as part of its recipe.