Adding Iodine as a new reactive element in RMG#1321
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1321 +/- ##
==========================================
+ Coverage 42.33% 42.33% +<.01%
==========================================
Files 168 168
Lines 27196 27207 +11
Branches 5248 5249 +1
==========================================
+ Hits 11514 11519 +5
- Misses 14937 14944 +7
+ Partials 745 744 -1
Continue to review full report at Codecov.
|
| ``Cl1s`` chlorine atom with three lone pairs and zero to one single bonds | ||
| *Iodine atom types* | ||
| ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ||
| ``I`` iodine atom with any local bond structure |
There was a problem hiding this comment.
It might not look important at first glance, but for this table to look good in our online documentation the spacings have to be precise. Note that the descriptions for Iodine aren't aligned with the other descriptions (one blank space needs to be added in each line before the description)
rmgpy/molecule/atomtype.py
Outdated
| @@ -242,7 +242,8 @@ def getFeatures(self): | |||
|
|
|||
There was a problem hiding this comment.
This is not because of the current PR, but there's an unnecessary blank line here. Could you remove it?
rmgpy/reaction.py
Outdated
| reactantChlorines = [sum([1 for atom in reactant.molecule[0].atoms if atom.isChlorine()]) for reactant in reactants] | ||
| productChlorines = [sum([1 for atom in product.molecule[0].atoms if atom.isChlorine()]) for product in products ] | ||
| reactantIodines = [sum([1 for atom in reactant.molecule[0].atoms if atom.isChlorine()]) for reactant in reactants] | ||
| productIodines = [sum([1 for atom in product.molecule[0].atoms if atom.isChlorine()]) for product in products ] |
There was a problem hiding this comment.
These should be spaced according to the above lines for better code readability
rmgpy/reaction.py
Outdated
|
|
||
| while len(reactants) > 1 and len(products) > 1: | ||
| self.pairs.append((reactants[-1][6], products[-1][6])) | ||
| self.pairs.append((reactants[-1][7], products[-1][7])) |
There was a problem hiding this comment.
Can you change [7] to [-1] here and below?
|
Thanks @alongd |
2 participants
Added I1s atomType along with respective atom test following the same procedure as Chlorine #1310