SKETCH-2378: Use getSGroupDataLabels() for DAT SGroup label positioning - #359
Draft
cdvonbargen wants to merge 2 commits into
Draft
SKETCH-2378: Use getSGroupDataLabels() for DAT SGroup label positioning#359cdvonbargen wants to merge 2 commits into
cdvonbargen wants to merge 2 commits into
Conversation
Replace manual FIELDDISP parsing in SGroupItem with the new getSGroupDataLabels() API from RDKit (rdkit/rdkit#9189), which properly handles absolute vs relative positioning and edge cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
@d-b-w -- when I did https://github.com/rdkit/rdkit/pull/9189/changes I didnt realize that MolDraw2DDetails.h would n't be accessible -- what is the right thing to do here? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replaces the manual FIELDDISP parsing in
SGroupItemwith the newMolDraw2D_detail::getSGroupDataLabels()API added in rdkit/rdkit#9189. The old code ignored the absolute/relative positioning flag and always treated FIELDDISP coordinates as relative to the first atom; the new API properly handles both cases along with edge cases like missing atoms or FIELDDISP entries.Build blocker: The pre-built RDKit package does not install
MolDraw2DDetails.has a public header, and theRDKit::MolDraw2Dcmake target has broken freetype paths from the build bot. This PR needs either: (a) the header added to the RDKit install set and the cmake target fixed, or (b) a local RDKit source build.Testing Done
Not yet compilable due to the RDKit packaging issue described above. Once resolved, verify with: