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

Jump to bottom

Clean ligand names while being set #474

Open
jthorton opened this issue Jan 31, 2025 · 4 comments · May be fixed by #480
Open

Clean ligand names while being set #474

jthorton opened this issue Jan 31, 2025 · 4 comments · May be fixed by #480
Assignees
@atravitz

Comments

@jthorton
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Ligand names with spaces cause issues downstream in openfe when we plan networks as folders and files are created based on ligand names.

Describe the solution you'd like

It would be great if we could remove any spaces in the name and replace them with - and remove any leading or trailing spaces.

Describe alternatives you've considered

We could do this specifically in the CLI in openfe. Still, I think it might be better to do this when creating a molecule or setting the name to ensure consistent behaviour across the stack.

Additional context

cc @IAlibay @hannahbaumann @atravitz
@jthorton jthorton self-assigned this Jan 31, 2025
@IAlibay
Copy link
Member

IAlibay commented Jan 31, 2025

I agree that normalisation should happen in gufe. Doing it only in the CLI (or anything in isolation) isn't a good idea.

@mikemhenry
Copy link
Contributor

We should also audit our code paths to make sure a ligand named foo/bar doesn't cause us issues OR we should raise an error invalid ligand name "foo/bar"

@jthorton jthorton linked a pull request Feb 12, 2025 that will close this issue
Open
2 tasks
@jameseastwood jameseastwood assigned atravitz and unassigned jthorton Feb 14, 2025
@dotsdl
Copy link
Member

dotsdl commented Feb 18, 2025

I would like to expand this proposal: let's standardize name as an optional property (can be None) common to all GufeTokenizables, with rules on what character sets are allowed for it. This would apply to ExplicitMoleculeComponent, Transformation, AlchemicalNetwork, etc.

@dotsdl
Copy link
Member

dotsdl commented Feb 18, 2025

From today's discussion: we concluded that we need to further characterize the problems users are encountering with the current permissiveness on names of ExplicitMoleculeComponents before we proceed with any proposed solution. It isn't clear that spaces in names, for example, are actually what is causing issues for some users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@atravitz atravitz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add name sanitization OpenFreeEnergy/gufe
5 participants
@dotsdl @mikemhenry @IAlibay @jthorton @atravitz