datajoint-python icon indicating copy to clipboard operation
datajoint-python copied to clipboard

Underscores in table names yield false positives for `get_master`

Open CBroz1 opened this issue 1 year ago • 2 comments
trafficstars

Bug Report

Description

Underscores are currently (a) permitted in table names and (b) used to determine master/part relationships. This can result in one table being falsely identified as the master of another.

Reproducibility

Include:

  • OS: Linux
  • Python: 3.9.15
  • MySQL: 8.0
  • MySQL Deployment Strategy: local docker
  • DataJoint Version: 0.14.1
  • Minimum number of steps to reliably reproduce the issue:
Python script
import datajoint as dj

schema = dj.schema("temp_issue")

@schema
class As(dj.Lookup):
    definition = """
    a: int
    """
    contents = [(1,), (2,), (3,)]

@schema
class As_Bs(dj.Lookup):
    definition = """
    a: int
    b: int
    """
    contents = [(1, 1), (2, 2), (3, 3)]

@schema
class As_Bs_Cs(dj.Manual):
    definition = """
    -> As_Bs
    c: int
    """

if __name__ == "__main__":
    As_Bs_Cs.drop()
  • Complete error stack as a result of evaluating the above steps
Error stack
---------------------------------------------------------------------------
DataJointError                            Traceback (most recent call last)
File ~/wrk/spyglass.git/tests/temp-issue.py:32
     25     definition = """
     26     -> As_Bs
     27     c: int
     28     """
     31 if __name__ == "__main__":
---> 32     As_Bs_Cs.drop()
     34     # DataJointError: Attempt to drop part table `temp_issue`.`as__bs__cs`
     35     # before dropping its master. Drop `temp_issue`.`as__bs` first.

File ~/wrk/datajoint-python/datajoint/table.py:721, in Table.drop(self)
    719     master = get_master(part)
    720     if master and master not in tables:
--> 721         raise DataJointError(
    722             "Attempt to drop part table {part} before dropping "
    723             "its master. Drop {master} first.".format(
    724                 part=part, master=master
    725             )
    726         )
    728 if config["safemode"]:
    729     for table in tables:

DataJointError: Attempt to drop part table `temp_issue`.`as__bs__cs` before dropping its master. Drop `temp_issue`.`as__bs` first.

Expected Behavior

Users should not be able to declare tables that will render false positives. I propose adding a check to the declare method that raises an error if underscores are used in the name to enforce the established Camel Case convention.

CBroz1 avatar Jan 17 '24 19:01 CBroz1

Thank you. Yes, we are not strongly enforcing naming conventions while relying on having them followed. We will enforce that the class names must be in strict CamelCase.

dimitri-yatsenko avatar Jan 26 '24 18:01 dimitri-yatsenko

Hi @CBroz1,

I'll resolve this issue next week. Apologies for the delay.

ethho avatar Feb 13 '24 20:02 ethho