Skip to content
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.

Sign up for GitHub

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

Test cofactor networks generated with the CLI #1048

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Test cofactor networks generated with the CLI #1048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
58ce1a4
make sure the cofactor is in the transformation
jthorton Dec 12, 2024
d8fc66e
fix cofactor name
jthorton Dec 12, 2024
03191ed
Merge branch 'main' into cofactor_tests
jthorton Dec 13, 2024
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
2 changes: 1 addition & 1 deletion openfe/setup/chemicalsystem_generator/easy_chemicalsystem_generator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def __call__(
RFEComponentLabels.PROTEIN: self.protein,
}
for i, c in enumerate(self.cofactors):
components.update({f'{RFEComponentLabels.COFACTOR}{i+1}': c})
components.update({f'{RFEComponentLabels.COFACTOR.value}{i+1}': c})
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the reason for using the value here instead of using the RFEComponentLabels object itself?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For some reason using it like this gives an unexpected label, the cofactor is called RFEComponentLabels.COFACTOR1 whereas the ligand and protein components are labelled ligand and protein as expected. See the error for how this looks, I guess its not super important to change as I don't think these labels are used for anything but it doesn't look nice and its not easy to pull out the cofactor with the inconsistent naming.

Copy link
Contributor

Choose a reason for hiding this comment

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

within abstract_chemicalsystem_generator.py, if you make the class inherit from StrEnum instead of both str and Enum (i.e.RFEComponentLabels(StrEnum)), it'll solve this!

It looks like StrEnum has some additional handling for string representation, but I don't quite understand the details.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ahhh thanks, though I think that the component keys should all be strings going by the type hint in gufe so maybe they should all be using .value?

if self.solvent is not None:
components.update({RFEComponentLabels.SOLVENT: self.solvent})
chem_sys = ChemicalSystem(
Expand Down
15 changes: 14 additions & 1 deletion openfecli/tests/commands/test_plan_rbfe_network.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@

import pytest
from importlib import resources
import os
import shutil
from click.testing import CliRunner

from openfecli.commands.plan_rbfe_network import (
plan_rbfe_network,
plan_rbfe_network_main,
)
from gufe import AlchemicalNetwork
from gufe.tokenization import JSON_HANDLER
import json


@pytest.fixture(scope='session')
Expand Down Expand Up @@ -148,6 +150,17 @@ def test_plan_rbfe_network_cofactors(eg5_files):
print(result.output)

assert result.exit_code == 0
# make sure the cofactor is in the transformations
network = AlchemicalNetwork.from_dict(
json.load(open("alchemicalNetwork/alchemicalNetwork.json"), cls=JSON_HANDLER.decoder)
)
for edge in network.edges:
if "protein" in edge.stateA.components:
assert "cofactor1" in edge.stateA.components
assert "cofactor1" in edge.stateB.components
else:
assert "cofactor1" not in edge.stateA.components
assert "cofactor1" not in edge.stateB.components


@pytest.fixture
Expand Down
Loading