extended AtomGroup with: bond orders, the ability to extend an atomgr… #1978
Conversation
…oup and __hash__ method
|
This sounds great too. Thank you |
|
Hello, thanks |
It seems to be a problem with the Bioexcel Covid-19 simulations database, which happens from time to time. I'd suggest rerunning the checks in a few days, at which point they will hopefully pass |
|
Do I have to do something to get a review done ? or is it just in the queue waiting to be reviewed ? |
|
It’s in the queue. We’re all busy but will get to it when we can |
| @@ -493,7 +525,7 @@ def _getCoords(self): | |||
| if self._coords is not None: | |||
| return self._coords[self._acsi] | |||
|
|
|||
| def setCoords(self, coords, label=''): | |||
| def setCoords(self, coords, label='', overwrite=False): | |||
There was a problem hiding this comment.
it could help to add docs for this
There was a problem hiding this comment.
Here I only exposed the overwrite=False optional argument present in the _setCoords() signature. _setCoords() is called by setCoords() but I could not pass overwrite=True. In _setCoords, overwrite can be used to force reshaping the _coords array, although the argument is not mentioned i the _setCoords() method doc string either. An alternative would have been for me to set ag1._coords to None prior to calling setCoords() but I felt exposing the argument was cleaner.
There was a problem hiding this comment.
Yes, but seeing as most users probably aren’t using _setCoords much it makes sense to add this to setCoords docs too
There was a problem hiding this comment.
I added the description in the doc string
|
Thank you James, no worries, I was just wondering about the process as this is my first attempt to do this.
…---------------------------------------------------------------------------------------
Michel F. Sanner
Professor of Computational Biology
Department of Integrated Structural and Computational Biology
Scripps Research, La Jolla, CA
[signature_2219072521]
From: James Krieger ***@***.***>
Date: Friday, November 8, 2024 at 12:21 AM
To: prody/ProDy ***@***.***>
Cc: Michel Sanner ***@***.***>, Author ***@***.***>
Subject: Re: [prody/ProDy] extended AtomGroup with: bond orders, the ability to extend an atomgr… (PR #1978)
It’s in the queue. We’re all busy but will get to it when we can
—
Reply to this email directly, view it on GitHub<#1978 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ALNO257ZGIJZBNPSWEKKQBTZ7RX7TAVCNFSM6AAAAABQN5ZUWGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRUGA4TQMBTG4>.
You are receiving this because you authored the thread.[https://github.com/notifications/beacon/ALNO25ZFBMLHWVMAUAUTKVDZ7RX7TA5CNFSM6AAAAABQN5ZUWGWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTUS34TPK.gif]Message ID: ***@***.***>
|
![https://github.com/notifications/beacon/ALNO25ZFBMLHWVMAUAUTKVDZ7RX7TA5CNFSM6AAAAABQN5ZUWGWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTUS34TPK.gif]Message](https://github.com/notifications/beacon/ALNO25ZFBMLHWVMAUAUTKVDZ7RX7TA5CNFSM6AAAAABQN5ZUWGWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTUS34TPK.gif]Message){kind=link}
|
You're welcome. I don't think we have any particular process. You'll see now that I have reviewed both your pull requests a first time and suggested several small changes. Thanks again for adding these! |
…requests from reviewer of PR
|
I still need to look through the changes properly |
…f the overwrite arguemnt\n- fixed typo in setBondOrders dock string
|
The PR with the ftp hot fixes is merged so hopefully the checks just pass now |
|
Great 😊
…---------------------------------------------------------------------------------------
Michel F. Sanner
Professor of Computational Biology
Department of Integrated Structural and Computational Biology
Scripps Research, La Jolla, CA
[signature_1610581944]
From: James Krieger ***@***.***>
Date: Thursday, November 14, 2024 at 11:40 PM
To: prody/ProDy ***@***.***>
Cc: Michel Sanner ***@***.***>, Author ***@***.***>
Subject: Re: [prody/ProDy] extended AtomGroup with: bond orders, the ability to extend an atomgr… (PR #1978)
The PR with the ftp hot fixes is merged so hopefully the checks just pass now
—
Reply to this email directly, view it on GitHub<#1978 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ALNO254JTJAN3NR6GD7ZOET2AWQM3AVCNFSM6AAAAABQN5ZUWGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINZYGE2DKNJZGU>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
you probably need to pull from the main branch |
|
You probably need more unit tests for the new behaviour, but otherwise it looks good now. |
Added and extend() method to the AtomGroup class. This method performs the same operation as add but without creating a new AtomGRoup instance. This was done by modifying add a little and exposing the overwrite=False argument in setCoords(). I use it when adding atoms to an atom group (e.g. protonating the molecule, adding missing atoms, or mutating amino acid side chains). Since i might have several selections referring to this AtomGroup I want it to remain the same.
I also added support for storing bond order information using the new slots _bonOrders and _bondIndex.
bond orders can be set when bonds are set using an optional argument ag.setBonds(bonds, bondOrders). or after bonds have been set ag.setBondOrders(bondOrders). the bonOrders argument has to be a list of integers of length len(self._bonds) containing the following values: 1: single, 2: double, 3:triple, 4:aromatic, 5:amide.
I also add hash to the AtomGtoup to enable using them as keys in dictionaries.