-
Notifications
You must be signed in to change notification settings - Fork 845
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
S-groups: PARENT field should reference index #3175
Comments
This is a bit more clear after reading through #3170 (Had a bit of trouble understanding this without looking at your last comments there) |
@ricrogz : does the last paragraph I just added help? |
greglandrum
added a commit
to greglandrum/rdkit
that referenced
this issue
May 18, 2020
greglandrum
added a commit
that referenced
this issue
May 19, 2020
* Progress on #3168 * Fixes #3167 * Fixes #3169 * deal with CBONDS too * test PATOMS * Fixes #3175 * a bit of code simplification and test updates still needs more testing * more testing * handle s-group hierarchy also a couple of other changes in response to the review * add forgotten test file * changes in response to review
Sorry, I missed this. Yeah, that helped. Somehow I missed that this was also about preserving the value of the "index" field on input. |
greglandrum
added a commit
that referenced
this issue
Jun 9, 2020
* Progress on #3168 * Fixes #3167 * Fixes #3169 * deal with CBONDS too * test PATOMS * Fixes #3175 * a bit of code simplification and test updates still needs more testing * more testing * handle s-group hierarchy also a couple of other changes in response to the review * add forgotten test file * changes in response to review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Here's what
index
says in the docs:and
PARENT
:Since we can avoid altering the input data here, I think it makes sense to do so and to preserve the
index
andPARENT
fields from SGroups in mol blocks. When we do that, the PARENT should reference anindex
property, not an index in the vector ofSubstanceGroup
s.The text was updated successfully, but these errors were encountered: