fix macro expansion when used as arguments#1796
fix macro expansion when used as arguments#1796zhonghua-wang wants to merge 4 commits intomolstar:masterfrom
Conversation
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.
dsehnal
left a comment
There was a problem hiding this comment.
Can you please update the headers of the files you modified and add yourself to the package.json author list?
| 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); |
There was a problem hiding this comment.
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)))
There was a problem hiding this comment.
Right, but would it be enough to just cache the old value and restore it after the inner query call?
There was a problem hiding this comment.
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;
}There was a problem hiding this comment.
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({}); |
There was a problem hiding this comment.
Oops, wonder how this happened :D
There was a problem hiding this comment.
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.
When macros like
sel.atom.resare 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 addsatom.sec-struct.ismacro for secondary structure testing and fixes indentation in symbol definitions.related issue: #1715
Actions
[Unreleased]section ofCHANGELOG.mdpackage.json'scontributorsdocs