Merge pull request #1618 from datajoint/fix/alter-declaration-context-2.3

fix(alter): resolve foreign keys in the table's declaration context (2.3)

Author: dimitri-yatsenko

src/datajoint/table.py

@@ -316,10 +316,23 @@ def _refresh_lineage(self, context=None):
     def alter(self, prompt=True, context=None):
         """
         Alter the table definition from self.definition
+
+        Parameters
+        ----------
+        prompt : bool, optional
+            If True (default), show the ALTER statement and ask for confirmation.
+        context : dict, optional
+            Namespace that resolves the definition's foreign key references.
+            Defaults to the table's declaration context, so the table alters the
+            same way wherever this is called from; a table without one uses the
+            caller's namespace.
         """
         if self.connection.in_transaction:
             raise DataJointError("Cannot update table declaration inside a transaction, e.g. from inside a populate/make call")
         if context is None:
+            context = self.declaration_context
+        if context is None:
+            # f_back is alter()'s caller, so this cannot move into a helper
             frame = inspect.currentframe().f_back
             context = dict(frame.f_globals, **frame.f_locals)
             del frame

tests/integration/test_alter.py

@@ -1,7 +1,11 @@
+import importlib
 import re
+import sys
+import time
 
 import pytest
 
+import datajoint as dj
 
 from tests import schema as schema_any_module
 from tests.schema_alter import LOCALS_ALTER, Experiment, Parent
@@ -52,3 +56,100 @@ def test_alter_part(self, schema_alter):
             table=Parent.Grandchild,
             attribute_sql=r"`grandchild_id` .* DEFAULT NULL[^,]*",
         )
+
+
+# A pipeline module that declares its tables at import, like the one in #1602.
+# Its schema is activated by the test, on the test's connection.
+PIPELINE_MODULE = '''
+import datajoint as dj
+
+schema = dj.Schema()
+
+
+@schema
+class Item(dj.Manual):
+    definition = """
+    item : int32
+    """
+
+
+@schema
+class Measurement(dj.Manual):
+    definition = """
+    -> Item
+    ---
+    value : float32
+    """
+
+    class Note(dj.Part):
+        definition = """
+        -> master
+        ---
+        note : varchar(64)
+        """
+'''
+
+
+@pytest.fixture
+def schema_by_backend(connection_by_backend, db_creds_by_backend):
+    """Create a fresh schema per test, parameterized across backends."""
+    backend = db_creds_by_backend["backend"]
+    test_id = str(int(time.time() * 1000))[-8:]
+    schema = dj.Schema(f"djtest_alter_{backend}_{test_id}"[:64], connection=connection_by_backend)
+    yield schema
+    schema.drop(prompt=False)
+
+
+@pytest.fixture
+def pipeline(schema_by_backend, tmp_path, monkeypatch):
+    """Import the pipeline module from tmp_path, its tables declared in the test schema."""
+    (tmp_path / "alter_pipeline.py").write_text(PIPELINE_MODULE)
+    monkeypatch.syspath_prepend(str(tmp_path))
+    try:
+        module = importlib.import_module("alter_pipeline")
+        module.schema.activate(schema_by_backend.database, connection=schema_by_backend.connection)
+        yield module
+    finally:
+        sys.modules.pop("alter_pipeline", None)
+
+
+# Each alter adds an attribute: changing an attribute's type emits MODIFY, which
+# PostgreSQL does not accept. Foreign keys resolve before any SQL is generated.
+
+
+def test_alter_from_another_module(pipeline):
+    """alter() resolves foreign keys in the declaration context, not in the caller's namespace (#1602)."""
+    # This module has no `Item`, so resolving `-> Item` here would fail
+    pipeline.Measurement.definition = """
+    -> Item
+    ---
+    value : float32
+    unit = "" : varchar(16)
+    """
+    pipeline.Measurement.alter(prompt=False)
+    assert pipeline.Measurement.heading.secondary_attributes == ["value", "unit"]
+
+
+def test_alter_part_from_another_module(pipeline):
+    """A Part table still resolves `-> master` in its declaration context."""
+    pipeline.Measurement.Note.definition = """
+    -> master
+    ---
+    note : varchar(64)
+    author = "" : varchar(32)
+    """
+    pipeline.Measurement.Note.alter(prompt=False)
+    assert pipeline.Measurement.Note.heading.secondary_attributes == ["note", "author"]
+
+
+def test_alter_explicit_context(pipeline):
+    """An explicit context takes precedence over the declaration context."""
+    # `Parent` resolves only in the explicit context
+    pipeline.Measurement.definition = """
+    -> Parent
+    ---
+    value : float32
+    unit = "" : varchar(16)
+    """
+    pipeline.Measurement.alter(prompt=False, context={"Parent": pipeline.Item})
+    assert pipeline.Measurement.heading.secondary_attributes == ["value", "unit"]