Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 7, 2025

Implementation Plan for Macro Feature

  • Explore repository structure and understand architecture
  • Review defcells implementation pattern
  • Review export functionality
  • Set up development environment
  • Create Macro and MacroImpl classes (similar to Cells/CellsImpl)
  • Create MacroMaker class (similar to CellsMaker)
  • Add defmacro function in api.py (analogous to defcells)
  • Add macros property and methods to Model/ModelImpl
  • Implement macro namespace with mx_model variable
  • Add macro serialization support (deferred - not required for MVP)
  • Add macro export support (_mx_macros.py)
  • Create comprehensive tests for macro functionality
  • Test macro creation and execution
  • Test macro export functionality
  • Run all tests and verify no regressions
  • Address all code review feedback
  • Fix validation and error handling consistency
  • Add comprehensive documentation to reference guide
  • Use _name instead of name property per feedback

Documentation Added

  • ✅ Added defmacro to Creating objects section in functions.rst
  • ✅ Added Model.macros to Model properties section in model.rst
  • ✅ Added Macro operations section with new_macro in model.rst
  • ✅ Created macro.rst documenting the Macro class
  • ✅ Added macro.rst to reference guide index

Changes Per Latest Feedback

  • ✅ Removed name property from BaseParent in _mx_sys.py
  • ✅ Updated all macros to use mx_model._name instead of mx_model.name
  • ✅ Updated all test files to use _name
  • ✅ Updated all docstrings to use _name
  • ✅ All tests pass (21 core macro tests + 7 export tests)

Summary

Successfully implemented and documented the Macro feature for modelx with:

  • 28 comprehensive tests (all passing)
  • Core functionality: defmacro decorator, macro execution, shared namespace
  • Export support: _mx_macros.py generation, seamless integration with exported models
  • Full Model integration: macros property, attribute access, deletion
  • Code quality: All code review issues addressed, proper validation, error handling
  • Documentation: Complete reference documentation including docstrings and RST files
  • Proper usage: Macros use _name attribute directly without property wrapper

The implementation is complete, tested, documented, and ready for use!

Original prompt

This section details on the original issue you should resolve

<issue_title>Implement Macro</issue_title>
<issue_description>Macros are Python functions that can be saved within a Model. With macros, users can have Python functions to run and manipulate the model together with the model.

All macros in a model share the same dedicated global namespace.
In the namespace, the model is defined as a global variable, mx_model as well as its model name.

defmacro is defined analogous to defcells as a decorate for creating a Macro object from the definition of a Python function:

>>> import modelx as mx

>>> m = mx.new_model('MyMacro')


>>> # The defmacro constractor without parameters defines a macro as the decorated function in the current model.
>>> @mx.defmacro    
... def get_model_name():
...     return mx_model.name    # or MyMacro.name


>>> # defmacro can take 'model' and 'name' arguments to specify the model and the name of the macro.
>>> @mx.defmacro(model=m, name='print_name')
... def print_model_name(message):   # the function name is replaced with the given name
...     print(f"{message} {get_model_name()}")  # macros in a model share the same global namespace so one macro can refer another macro.

Macros can be executed by calling them. They can be called as model's attributes:

>>> m.get_model_name()
'MyMacro'

>>> m.print_name("This model is")
This model is MyMacro

Analogous to Model.refs and Model.spaces, Model.macros returns all macros as a mapping:

>>> m.macros  # Returns a mapping of the macro names to Macro objects.
{'get_model_name': <Macro MyMacro.get_model_name>, 'print_name': <Macro MyMacro.print_name>}

When a model is exported, its macros are exported and defined in _mx_macros.py as normal functions available in the top module. This allows the macros to be able to run on both the modelx model and its exported model.

>>> m.export('my_macro')   # Exports the model as a self-contained Python pacakage

In _mx_macros.py:

from ._mx_model import (mx_model, MyMacro)

def get_model_name():
    return mx_model.name    # or MyMacro.name

def print_name():   # the function name is replaced with the given name
    print(f"{message} {get_model_name()}")

In __init__.py:

from ._mx_model import (mx_model, MyMacro)
from ._mx_macros import (get_model_name, print_name)

</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@fumitoh fumitoh marked this pull request as ready for review December 7, 2025 12:50
Copilot AI and others added 4 commits December 7, 2025 12:55
…del integration

Co-authored-by: fumitoh <8450892+fumitoh@users.noreply.github.com>
Co-authored-by: fumitoh <8450892+fumitoh@users.noreply.github.com>
…tion

Co-authored-by: fumitoh <8450892+fumitoh@users.noreply.github.com>
Co-authored-by: fumitoh <8450892+fumitoh@users.noreply.github.com>
Copilot AI changed the title [WIP] Add macro implementation for Python functions in models Implement Macro feature for saving Python functions within Models Dec 7, 2025
Copilot AI requested a review from fumitoh December 7, 2025 13:12
@fumitoh
Copy link
Owner

fumitoh commented Dec 8, 2025

@copilot
Document Macro functionality by including the docstrings of relevant functions, methods, classes as follows:

  • List defmacro in the Creating objects section in functions.rst
  • List Model.macros in the Model properties section in model.rst
  • Add Macro operatons section in model.rst and list new_macro
  • Add macro.rst in the doc/source/refernce dir and document the Macro class there

- List defmacro in Creating objects section in functions.rst
- Add Model.macros to Model properties in model.rst
- Add Macro operations section with new_macro in model.rst
- Create macro.rst documenting the Macro class
- Add macro.rst to index.rst toctree

Co-authored-by: fumitoh <8450892+fumitoh@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 8, 2025

@copilot
Document Macro functionality by including the docstrings of relevant functions, methods, classes as follows:

  • List defmacro in the Creating objects section in functions.rst
    ...

Documentation has been added in commit f1e7719:

✅ Listed defmacro in Creating objects section in functions.rst
✅ Added Model.macros to Model properties section in model.rst
✅ Added Macro operations section with new_macro in model.rst
✅ Created macro.rst documenting the Macro class with formula and parent properties
✅ Added macro.rst to the reference guide index.rst

All docstrings were already in place from the implementation, so the RST files now properly reference and document the Macro functionality.

@fumitoh
Copy link
Owner

fumitoh commented Dec 8, 2025

@copilot The name proeprty should not be used in macros. Model._name should be used in place for Model.name. Do not add name property to BaseParent in _mx_sys.py

- Remove name property from BaseParent in _mx_sys.py
- Update all macro code and tests to use mx_model._name instead of mx_model.name
- Update docstrings in api.py, macro.py, and model.py to use _name
- All tests pass successfully

Co-authored-by: fumitoh <8450892+fumitoh@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 8, 2025

@copilot The name proeprty should not be used in macros. Model._name should be used in place for Model.name. Do not add name property to BaseParent in _mx_sys.py

Changed in commit c600163:

✅ Removed name property from BaseParent in _mx_sys.py
✅ Updated all macros to use mx_model._name instead of mx_model.name
✅ Updated all tests (21 core + 7 export tests) to use _name
✅ Updated all docstrings in api.py, macro.py, and model.py to use _name

All tests pass successfully.

@fumitoh fumitoh merged commit e01c237 into main Dec 8, 2025
21 of 22 checks passed
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.

Implement Macro

2 participants