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

Suggestion: Make structural elements mutable #22

Open
amaurypm opened this issue Jul 2, 2020 · 7 comments
Open

Suggestion: Make structural elements mutable #22

amaurypm opened this issue Jul 2, 2020 · 7 comments

Comments

@amaurypm
Copy link

amaurypm commented Jul 2, 2020

I just was trying to use BioStructures to solve a problem which required making copies of chains, rename them, alter atom serial numbers and other modifications to structural elements. In its current implementation only the atom coordinates can be modified. This package is great, but I think it could be much more useful if the corresponding structures are declared as mutable, and I don't see any downside on doing that.
It would be great to be able to modify atoms, residues and chains and create new models/structures with them.
Best regards,
Amaury

@jgreener64
Copy link
Member

Thanks for trying out the package. I agree that mutable elements would be great. Sadly, it's a little more complex than adding mutable in front of struct. Firstly, you lose some speed, though I don't know how much. I could benchmark that quite easily actually.

More importantly though, you can't just change properties directly and expect the parent/child relationships to work. For example, if you relabel a chain you would need to modify the chain dictionary in the parent model.

I think the right way to do it would be to have setters (chainid! etc.) that create the relevant new immutable objects, modify existing objects as required, and throw errors when a name clash is found. That has been on the todo list for a while but it is a fair amount of work, and I'll admit I didn't have so much use for it myself. If I was writing the package from scratch I would build this in from the start and it would probably be easier.

The lowest-hanging fruit would be to do chainid! as an example and see how it goes. I might have a go at some point.

@amaurypm
Copy link
Author

amaurypm commented Jul 2, 2020

Thank you for your response. I suspected it won't be so easy as making the structures mutable, and I was thinking in setters, as you explained. I guess for now I'll try to implement the creation of new elements from existing ones in my code, copying what I want to preserve and modifying the rest.

@jgreener64
Copy link
Member

Someone else asked about this on Discourse: https://discourse.julialang.org/t/how-can-i-set-a-specific-chain-name-for-a-structure-biostrcuture-package/50211

Making the structs mutable doesn't actually lose any speed, so I'll try and get some setters coded.

@gusennan
Copy link
Contributor

gusennan commented Sep 30, 2021

I have the same use case as @amaurypm and have ended up here after going down the same route that he has. Do you want some help on this?

@jgreener64
Copy link
Member

That would be great, I haven't had time myself. I am happy to help and to review PRs, I would recommend seeing how chainid! goes first.

gusennan added a commit to gusennan/BioStructures.jl that referenced this issue Oct 21, 2021
Adds two function for assigning the chainid, chainid!(chain, new_id) and
chainid!(residue, new_id). For GitHub issue BioJulia#22.
@gusennan
Copy link
Contributor

gusennan commented Oct 21, 2021

That would be great, I haven't had time myself. I am happy to help and to review PRs, I would recommend seeing how chainid! goes first.

I took a stab at it. It was a bit more complicated than I thought it would be :)

jgreener64 pushed a commit that referenced this issue Oct 26, 2021
* Allow chain id to be assigned with chainid!()

Adds two function for assigning the chainid, chainid!(chain, new_id) and
chainid!(residue, new_id). For GitHub issue #22.

* Create a PDBConsistencyException

- Throw PDBConsistencyException when trying to change a chain's ID to an
ID that already exists.
- Throw PDBConsistencyException when moving a residue to a chain that
already has a residue with that number.
- Update the model's chain dictionary when changing a chain ID.

* Rename PDBConsistencyException to PDBConsistencyError

- Added a test for ensuring KeyError thrown when ChainID changed to
something else.
@jgreener64
Copy link
Member

Thanks to @gusennan the chainid! function is available as of v1.1.0.

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

3 participants