Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #740 +/- ##
==========================================
- Coverage 73.80% 73.79% -0.01%
==========================================
Files 99 99
Lines 27352 27361 +9
Branches 5718 5720 +2
==========================================
+ Hits 20187 20191 +4
- Misses 5738 5743 +5
Partials 1427 1427
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
If generate_resonance_structures returns None, then still pass the isomorphism check.
This is done as per the schema by other internal coordinates
alongd
left a comment
There was a problem hiding this comment.
Thanks. Please see two comments
| @@ -1117,8 +1117,12 @@ def r_cut_p_cut_isomorphic(reactant, product): | |||
| bool: ``True`` if they are isomorphic, ``False`` otherwise. | |||
| """ | |||
| res1 = generate_resonance_structures(object_=reactant.mol.copy(deep=True), save_order = True) | |||
There was a problem hiding this comment.
I would recommend to modify the generate_resonance_structures func so it always returns at least the original Molecule. Please see where else this func is used and whether this change will be appropriate
| except VectorsError: | ||
| # Trying to map a linear torsion | ||
| # but the torsion is undefined, since v1 x v2 = 0 if v1 || v2 | ||
| # using other internal coordinates to map the hydrogens. |
There was a problem hiding this comment.
does this fix solve the issue or skips it? We could consider using a different atom instead of the last atom in the torsion
Two small fixes for the atom mapping module.