Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 10, 2025

When deleting a macro via del model.macro_name, it was removed from _macros dict but persisted in _macro_namespace, causing stale references.

Changes

  • modelx/core/model.py: Modified del_macro() to also remove the macro from _macro_namespace when present
  • modelx/tests/core/model/test_macro.py: Updated test_macro_delete() to verify namespace cleanup

Example

import modelx as mx

m = mx.new_model('MyModel')

@mx.defmacro
def get_model_name():
    return mx_model._name

m.get_model_name()  # Creates namespace
del m.get_model_name

# Now passes (previously failed)
assert 'get_model_name' not in m._impl._macro_namespace

The fix handles the lazy initialization of _macro_namespace (only created on first macro execution) by checking for None before attempting deletion.

Original prompt

This section details on the original issue you should resolve

<issue_title>Bug: Macros not removed from the namespace when they are removed</issue_title>
<issue_description>Fix the bug that macros are not removed from Model._macro_namespace when they are deleted.

import modelx as mx

m = mx.new_model('MyModel')

@mx.defmacro
def get_model_name():
    return mx_model._name

m.get_model_name()

del m.get_model_name

assert 'get_model_name' not in m._impl._macro_namespace   # This should pass but it raises AssertionError
```</issue_description>

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

<comments>
</comments>

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

Co-authored-by: fumitoh <8450892+fumitoh@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix macros not removed from namespace on delete Fix: Remove macros from namespace when deleted Dec 10, 2025
Copilot AI requested a review from fumitoh December 10, 2025 15:54
@fumitoh fumitoh marked this pull request as ready for review December 10, 2025 16:11
@fumitoh fumitoh merged commit 438a844 into main Dec 10, 2025
42 of 44 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.

Bug: Macros not removed from the namespace when they are removed

2 participants