Apply suggestions from code review

Author: rly

datajoint/user_tables.py

@@ -2,7 +2,6 @@
 Hosts the table tiers, user relations should be derived from.
 """
 
-import inspect
 from .table import Table
 from .autopopulate import AutoPopulate
 from .utils import from_camel_case, ClassProperty
@@ -189,14 +188,5 @@ def drop(self, force=False):
             raise DataJointError('Cannot drop a Part directly.  Delete from master instead')
 
     def alter(self, prompt=True, context=None):
-        """
-        Alter the table definition from self.definition
-        """
-        # map "master" keyword to master table in context
-        if context is None:
-            frame = inspect.currentframe().f_back
-            context = dict(frame.f_globals, **frame.f_locals)
-            del frame
-        if self.master:
-            context['master'] = self.master
-        super().alter(prompt, context)
+        # when there is no context, map "master" keyword to master table
+        super().alter(prompt=prompt, context=context or self.declaration_context)

docs-parts/intro/Releases_lang1.rst

@@ -15,7 +15,6 @@
 * Bugfix - Fix assertion error when performing a union into a join (#930) PR #967
 * Bugfix - Fix regression issue with `DISTINCT` clause and `GROUP_BY` (#914) PR #963
 * Update `~jobs.error_stack` from blob to mediumblob to allow error stacks >64kB in jobs (#984) PR #986
-* Bugfix - Fix error when altering a part table that uses the "master" keyword (#936) PR #991
 
 0.13.2 -- May 7, 2021
 ----------------------

tests/test_alter.py

@@ -1,4 +1,5 @@
 from nose.tools import assert_equal, assert_not_equal
+import re
 from .schema import *
 
 
@@ -28,6 +29,33 @@ class Experiment(dj.Imported):
     """
 
 
+@schema
+class Parent(dj.Manual):
+    definition = """
+    parent_id: int
+    """
+
+    class Child(dj.Part):
+        definition = """
+        -> Parent
+        """
+        definition_new = """
+        -> master
+        ---
+        child_id=null: int
+        """
+
+    class Grandchild(dj.Part):
+        definition = """
+        -> master.Child
+        """
+        definition_new = """
+        -> master.Child
+        ---
+        grandchild_id=null: int
+        """
+
+
 def test_alter():
     original = schema.connection.query("SHOW CREATE TABLE " + Experiment.full_table_name).fetchone()[1]
     Experiment.definition = Experiment.definition1
@@ -41,24 +69,23 @@ def test_alter():
     assert_equal(original, restored)
 
 
-@schema
-class AlterMaster(dj.Manual):
-    definition = """
-    master_id : int
-    """
+def test_alter_part():
+    # https://github.com/datajoint/datajoint-python/issues/936
 
-    class AlterPart(dj.Part):
-        definition = """
-        -> master
-        """
-
-        definition1 = """
-        -> AlterMaster
-        """
+    def verify_alter(table, attribute_sql):
+        definition_original = schema.connection.query(
+            f"SHOW CREATE TABLE {table.full_table_name}"
+        ).fetchone()[1]
+        table.definition = table.definition_new
+        table.alter(prompt=False)
+        definition_new = schema.connection.query(
+            f"SHOW CREATE TABLE {table.full_table_name}"
+        ).fetchone()[1]
+        assert (
+            re.sub(f"{attribute_sql},\n  ", "", definition_new) == definition_original
+        )
 
-def test_alter_part_master_keyword():
-    original = schema.connection.query("SHOW CREATE TABLE " + AlterMaster.AlterPart.full_table_name).fetchone()[1]
-    AlterMaster.AlterPart.definition = AlterMaster.AlterPart.definition1
-    AlterMaster.AlterPart.alter(prompt=False)
-    altered = schema.connection.query("SHOW CREATE TABLE " + AlterMaster.AlterPart.full_table_name).fetchone()[1]
-    assert_equal(original, altered)
+    verify_alter(table=Parent.Child, attribute_sql="`child_id` .* DEFAULT NULL")
+    verify_alter(
+        table=Parent.Grandchild, attribute_sql="`grandchild_id` .* DEFAULT NULL"
+    )