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.

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

Support inferBonds parameter in PDB files #1092

Open
Yoshanuikabundi opened this issue Dec 7, 2023 · 4 comments
Open

Support inferBonds parameter in PDB files #1092

Yoshanuikabundi opened this issue Dec 7, 2023 · 4 comments

Comments

@Yoshanuikabundi
Copy link
Contributor

Upstream NGL has recently added support for configuring which bonds are inferred from interatomic distances in PDB files: nglviewer/ngl#977. This was released on Node in 2.2.0

At OpenFF, we'd love to be able to take advantage of this option in our visualization methods. Would you accept a PR to add this functionality?

I think it would involve:

  • Updating the version of NGL used by NGLView to at least 2.2.0
    • NGLView seems to be on "2.0.0-dev.39" - is there a reason we're on a pre-release from 3 years ago? Or is this a minimum requirement and Node will automatically use the latest version? Is there a way to see which version is being used in a particular browser session?
  • Ideally, adding a way to set PDB loader parameters from Structure subclasses
    • it seems that the Structure.params attribute would be for this, but it doesn't seem to get used anywhere. Or am I missing something?
    • in a pinch, we could use Widget._remote_call to load the PDB by hand if necessary, but I would think first-class support for this configuration object would be widely useful.

Thanks!

@hainm
Copy link
Collaborator

hainm commented Dec 7, 2023

Hi @Yoshanuikabundi

Would you accept a PR to add this functionality?

yeah, PRs are always very welcome

Updating the version of NGL used by NGLView to at least 2.2.0

Please continue my work here (the build passed on my macbook but failing. I guess I am using newer lab version (or so on)
#1086

NGLView seems to be on "2.0.0-dev.39" - is there a reason we're on a pre-release from 3 years ago?

no, it is just I am too lazy to check and no one ever asked.

Ideally, adding a way to set PDB loader parameters from Structure subclasses

Agree.

it seems that the Structure.params attribute would be for this, but it doesn't seem to get used anywhere. Or am I missing something?

No you're not. I think we don't use it. But I am open for suggestion.
I think the params will be passed in this _load_data function:

The API can be

nv.show_file('abc.pdb', infer_bonds=True)

@hainm
Copy link
Collaborator

hainm commented Dec 7, 2023

But please use 2 PRs

  • one for upgrading NGL version
  • another one for API update

thanks

@hainm
Copy link
Collaborator

hainm commented Jan 14, 2024

  • Updating the version of NGL used by NGLView to at least 2.2.0

@Yoshanuikabundi updated in nglview 3.1.0

@Yoshanuikabundi
Copy link
Contributor Author

Oh awesome, thanks @hainm! It's going to be a while before I can get to contributing to this - hopefully I'll be able to get to it in March.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants