Skip to content

#3604 Add api methods to interact with atoms and bonds in s-groups - #3625

Merged
AlexanderSavelyev merged 39 commits into
masterfrom
3604-sgroups-api
Jun 12, 2026
Merged

#3604 Add api methods to interact with atoms and bonds in s-groups#3625
AlexanderSavelyev merged 39 commits into
masterfrom
3604-sgroups-api

Conversation

@even1024

Copy link
Copy Markdown
Contributor

Generic request

  • PR name follows the pattern #1234 – issue name
  • branch name does not contain '#'
  • base branch (master or release/xx) is correct
  • PR is linked with the issue
  • task status changed to "Code review"
  • code follows product standards
  • regression tests updated

@AlexanderSavelyev AlexanderSavelyev linked an issue Apr 30, 2026 that may be closed by this pull request
Comment thread core/indigo-core/molecule/molecule_sgroups.h Outdated
Comment thread core/indigo-core/molecule/src/molfile_saver.cpp Outdated
Comment thread core/indigo-core/molecule/src/molfile_saver.cpp Outdated
Comment thread api/tests/integration/tests/basic/3604_sgroup_atoms_bonds.py
Comment thread core/indigo-core/layout/src/metalayout.cpp Outdated
Comment thread api/java/indigo/src/main/java/com/epam/indigo/IndigoObject.java
Comment thread api/dotnet/src/IndigoObject.cs Outdated
Comment thread api/tests/integration/ref/basic/sgroups_basic.py.out

if ((sg_child.parent_group != 0) && (sg_child.parent_group == sgroup.original_group))
_addSgroupElement(sg, mol, sg_child);
if (child_entry.write_parent == write_index)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this mean?

Comment thread api/tests/integration/tests/formats/ref/macro/sa-mono.cml
{
DataSGroup& group = (DataSGroup&)sg;
if (!group.relative)
if (!group.relative && group.display_pos.hasValue())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be just write

}

operator const T&() const
T& get()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make verification on value

Comment thread core/render2d/src/render_internal.cpp Outdated
TextItem& index = _data.textitems[tiIndex];
index.fontsize = FONT_SIZE_ATTR;
bprintf(index.text, "%d", group.multiplier);
bprintf(index.text, "%d", group.multiplier.get());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to add check

Comment thread core/render2d/src/render_internal.cpp Outdated
{
const Superatom& group = (Superatom&)sgroup;
Vec3f displayPosition = group.display_position;
Vec3f displayPosition = group.display_position.get();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to add check

{
DataSGroup& group = (DataSGroup&)sg;
if (!group.relative)
if (!group.relative && group.display_pos.hasValue())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is write, should not check for value

@@ -340,7 +340,7 @@ void CmfSaver::_encodeExtSection(Molecule& mol, const Mapping& mapping)
_encodeString(sd.data);
// Pack detached, relative, display_units, and sd.dasp_pos into one byte
if (sd.dasp_pos < 0 || sd.dasp_pos > 9)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add check

_encodeUIntArray(sm.parent_atoms, *mapping.atom_mapping);
if (sm.multiplier < 0)
throw Error("internal error: SGroup multiplier is negative: %d", sm.multiplier);
throw Error("internal error: SGroup multiplier is negative: %d", sm.multiplier.get());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add check

{
for (i = _mol->sgroups.begin(); i != _mol->sgroups.end(); i = _mol->sgroups.next(i))
auto entries = getOrderedSGroups(_mol->sgroups);
for (auto& entry : entries)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to sgroupEntry

std::vector<int> sgs_mapping(pool_end, 0);

// Roots first (parent_group == 0 or not set)
int iw = 1;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename

{
SGroup& sg = sgroups.getSGroup(i);
int pg = sg.parent_group.hasValue() ? sg.parent_group.get() : 0;
if (pg == 0)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename

std::set<int> added_indices;

// Add roots first
for (auto& e : all_entries)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename

struct SGroupWriteEntry
{
int pool_idx; // original pool index in mol.sgroups
int write_index; // sequential 1,2,3... for CTFile output

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename write_index to index

@@ -565,7 +565,7 @@ void MoleculeRenderInternal::_initSGroups(Tree& sgroups, Rect2f parent)
TextItem& ti = _data.textitems[tii];
if (group.tag != ' ')
{

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add check

for (int i : pool_indices)
{
SGroup& sg = sgroups.getSGroup(i);
int key = (sg.index != 0) ? sg.index : sgs_mapping[i];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

potential problem

@AlexanderSavelyev AlexanderSavelyev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix tests

@even1024
even1024 force-pushed the 3604-sgroups-api branch from 3bb039d to e84faa7 Compare June 9, 2026 14:04
@even1024
even1024 force-pushed the 3604-sgroups-api branch from e84faa7 to 852d265 Compare June 9, 2026 20:35
@AlexanderSavelyev
AlexanderSavelyev merged commit 6cc2697 into master Jun 12, 2026
64 checks passed
@AlexanderSavelyev
AlexanderSavelyev deleted the 3604-sgroups-api branch June 12, 2026 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add api methods to interact with atoms and bonds in s-groups

2 participants