Skip to content

fix macro expansion when used as arguments#1796

Open
zhonghua-wang wants to merge 4 commits intomolstar:masterfrom
zhonghua-wang:master
Open

fix macro expansion when used as arguments#1796
zhonghua-wang wants to merge 4 commits intomolstar:masterfrom
zhonghua-wang:master

Conversation

@zhonghua-wang
Copy link
Copy Markdown

@zhonghua-wang zhonghua-wang commented Mar 23, 2026

When macros like sel.atom.res are used as arguments (e.g., in (atom.set.count-query sel.atom.res)), the transpiler throw an empty array [] to the macro's translate function, causing it to fail. This change passes an empty object {} instead, allowing macros to expand with their default arguments. Also adds atom.sec-struct.is macro for secondary structure testing and fixes indentation in symbol definitions.

related issue: #1715

Actions

  • Added description of changes to the [Unreleased] section of CHANGELOG.md
  • Updated headers of modified files
  • Added my name to package.json's contributors
  • (Optional but encouraged) Improved documentation in docs

When macros like `sel.atom.res` are used as arguments (e.g., in
`(atom.set.count-query sel.atom.res)`), the transpiler was passing
an empty array `[]` to the macro's translate function, causing it to
fail. This change passes an empty object `{}` instead, allowing
macros to expand with their default arguments.

Also adds `atom.sec-struct.is` macro for secondary structure testing
and fixes indentation in symbol definitions.
Copy link
Copy Markdown
Member

@dsehnal dsehnal left a comment

Choose a reason for hiding this comment

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

Can you please update the headers of the files you modified and add yourself to the package.json author list?

Comment on lines +25 to +32
const { currentStructure } = ctx;
if (!currentStructure) {
const sel = query(ctx);
return StructureSelection.structureCount(sel);
}
const tempCtx = new QueryContext(currentStructure, { currentSelection: ctx.currentSelection });
tempCtx.currentStructure = currentStructure;
const sel = query(tempCtx);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The newly added code provides a clean context for the execution of nested queries, which preserves currentSelection from parent context, as a result, each countQuery execution gets its own isolated context, ensuring correct counting behavior.
e.g.

  (sel.atom.atom-groups
  :residue-test (atom.sec-struct.is helix)
  :group-by atom.key.sec-struct)
  :test (<= 30 (atom.set.count-query sel.atom.res)))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right, but would it be enough to just cache the old value and restore it after the inner query call?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

As I understand it, with this approach, the nested query will use currentSelection rather than the full Structure in the structureCount function.

    export function structureCount(sel: StructureSelection) {
        if (isSingleton(sel)) return sel.structure.elementCount;
        return sel.structures.length;
    }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No, the "current selection" is only accessed via MolScript.internal.generator.current, which has a different use case.

There is no :test (<= 30 (atom.set.count-query sel.atom.res)) in sel.atom.atom-groups. You need to use sel.atom.pick to filter out the helices you are looking for.

const s = SymbolMap[expr.name]!;
if (s.kind === 'alias') return Expression.Symbol(SymbolMap[expr.name]!.symbol.id);
throw s.translate([]);
return s.translate({});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oops, wonder how this happened :D

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have realised this is actually working as intended. You are supposed to "call" the function with (atom.key.sec-struct) (the extra ()) and then things should work as expected.

Symbols are not supposed to be applied automatically.

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.

2 participants