From a152006eba1bcbe22c4fe81b761f32319c9610c1 Mon Sep 17 00:00:00 2001 From: Ethan Ho <53266718+ethho@users.noreply.github.com> Date: Wed, 21 Feb 2024 16:08:47 +0000 Subject: [PATCH 01/11] Failing pytest reproducing #1150 pytest -sv tests/test_declare.py::test_table_name_with_underscores --- tests/test_declare.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_declare.py b/tests/test_declare.py index dfca54c2..74670ad9 100644 --- a/tests/test_declare.py +++ b/tests/test_declare.py @@ -337,3 +337,25 @@ class WithSuchALongPartNameThatItCrashesMySQL(dj.Part): with pytest.raises(dj.DataJointError): schema_any(WhyWouldAnyoneCreateATableNameThisLong) + + +def test_table_name_with_underscores(schema_any): + """ + Test issue #1150 -- Reject table names containing underscores. Tables should be in strict + CamelCase. + """ + + class TableNoUnderscores(dj.Manual): + definition = """ + id : int + """ + + class Table_With_Underscores(dj.Manual): + definition = """ + id : int + """ + + schema_any(TableNoUnderscores) + with pytest.raises(dj.DataJointError, match="CamelCase") as e: + schema_any(Table_With_Underscores) + From bb529bd319746d7741c0f5ad28328883210c0c8f Mon Sep 17 00:00:00 2001 From: Ethan Ho <53266718+ethho@users.noreply.github.com> Date: Wed, 21 Feb 2024 16:29:42 +0000 Subject: [PATCH 02/11] feat: enforce strict CamelCase for Table names --- datajoint/table.py | 12 ++++++++++++ tests/test_declare.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/datajoint/table.py b/datajoint/table.py index 251ff583..5cffab70 100644 --- a/datajoint/table.py +++ b/datajoint/table.py @@ -75,6 +75,10 @@ class Table(QueryExpression): def table_name(self): return self._table_name + @property + def class_name(self): + return self.__class__.__name__ + @property def definition(self): raise NotImplementedError( @@ -93,6 +97,14 @@ def declare(self, context=None): "Cannot declare new tables inside a transaction, " "e.g. from inside a populate/make call" ) + # Enforce strict CamelCase #1150 + if "_" in self.class_name: + raise DataJointError( + "Table with class name `{name}` contains an underscore. ".format( + name=self.class_name + ) + + "Classes defining tables should be formatted in strict CamelCase." + ) sql, external_stores = declare(self.full_table_name, self.definition, context) sql = sql.format(database=self.database) try: diff --git a/tests/test_declare.py b/tests/test_declare.py index 74670ad9..ef1a0fec 100644 --- a/tests/test_declare.py +++ b/tests/test_declare.py @@ -356,6 +356,6 @@ class Table_With_Underscores(dj.Manual): """ schema_any(TableNoUnderscores) - with pytest.raises(dj.DataJointError, match="CamelCase") as e: + with pytest.raises(dj.DataJointError, match="strict CamelCase") as e: schema_any(Table_With_Underscores) From a9470b5516d5cef6da46deeaaf6ff6511ee2c6a3 Mon Sep 17 00:00:00 2001 From: Ethan Ho <53266718+ethho@users.noreply.github.com> Date: Thu, 21 Mar 2024 21:16:24 +0000 Subject: [PATCH 03/11] Enforce strict CamelCase for Table subclasses Per @dimitri-yatsenko suggestion. --- datajoint/utils.py | 29 ++++++++++++++++++++++++++++- tests/test_utils.py | 15 ++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/datajoint/utils.py b/datajoint/utils.py index adf16091..27ed64ce 100644 --- a/datajoint/utils.py +++ b/datajoint/utils.py @@ -53,6 +53,33 @@ def get_master(full_table_name: str) -> str: return match["master"] + "`" if match else "" +def contains_non_ascii_char(s): + """ + Check if a string contains non-ASCII characters. + + :param s: string to check + :returns: True if the string contains any non-ASCII characters, False otherwise + Example: + >>> contains_non_ascii_char("Hello") # returns False + >>> contains_non_ascii_char("HelloΣ") # returns True + """ + return any(ord(c) > 127 for c in s) + + +def is_camel_case(s): + """ + Check if a string is in CamelCase notation. + + :param s: string to check + :returns: True if the string is in CamelCase notation, False otherwise + Example: + >>> is_camel_case("TableName") # returns True + >>> is_camel_case("table_name") # returns False + """ + # return re.match(r"^[A-Z][a-z0-9]+(?:[A-Z][a-z0-9]+)*$", s) is not None + return re.match(r"[A-Z][a-zA-Z0-9]*", s) is not None and not contains_non_ascii_char(s) + + def to_camel_case(s): """ Convert names with under score (_) separation into camel case names. @@ -82,7 +109,7 @@ def from_camel_case(s): def convert(match): return ("_" if match.groups()[0] else "") + match.group(0).lower() - if not re.match(r"[A-Z][a-zA-Z0-9]*", s): + if not is_camel_case(s): raise DataJointError( "ClassName must be alphanumeric in CamelCase, begin with a capital letter" ) diff --git a/tests/test_utils.py b/tests/test_utils.py index 619d4d16..6697df84 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -3,10 +3,23 @@ """ from datajoint import DataJointError -from datajoint.utils import from_camel_case, to_camel_case +from datajoint.utils import from_camel_case, to_camel_case, is_camel_case, contains_non_ascii_char import pytest +def test_is_camel_case(): + assert is_camel_case("AllGroups") + assert not is_camel_case("allGroups") + assert not is_camel_case("repNames") + assert not is_camel_case("10_all") + assert not is_camel_case("hello world") + assert not is_camel_case("#baisc_names") + assert not is_camel_case("alphaBeta") + non_ascii_class_name = "TestΣ" + assert contains_non_ascii_char(non_ascii_class_name) + assert not is_camel_case(non_ascii_class_name) + + def test_from_camel_case(): assert from_camel_case("AllGroups") == "all_groups" with pytest.raises(DataJointError): From 1cbfa45b9207f7c83655e7b4e113f0a790bef898 Mon Sep 17 00:00:00 2001 From: Ethan Ho <53266718+ethho@users.noreply.github.com> Date: Thu, 21 Mar 2024 21:17:34 +0000 Subject: [PATCH 04/11] Format with black --- datajoint/table.py | 4 ++-- datajoint/utils.py | 4 +++- tests/test_declare.py | 1 - tests/test_utils.py | 7 ++++++- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/datajoint/table.py b/datajoint/table.py index 7acd3bef..cbc75e99 100644 --- a/datajoint/table.py +++ b/datajoint/table.py @@ -102,8 +102,8 @@ def declare(self, context=None): raise DataJointError( "Table with class name `{name}` contains an underscore. ".format( name=self.class_name - ) + - "Classes defining tables should be formatted in strict CamelCase." + ) + + "Classes defining tables should be formatted in strict CamelCase." ) sql, external_stores = declare(self.full_table_name, self.definition, context) sql = sql.format(database=self.database) diff --git a/datajoint/utils.py b/datajoint/utils.py index 27ed64ce..b0dcf6a7 100644 --- a/datajoint/utils.py +++ b/datajoint/utils.py @@ -77,7 +77,9 @@ def is_camel_case(s): >>> is_camel_case("table_name") # returns False """ # return re.match(r"^[A-Z][a-z0-9]+(?:[A-Z][a-z0-9]+)*$", s) is not None - return re.match(r"[A-Z][a-zA-Z0-9]*", s) is not None and not contains_non_ascii_char(s) + return re.match( + r"[A-Z][a-zA-Z0-9]*", s + ) is not None and not contains_non_ascii_char(s) def to_camel_case(s): diff --git a/tests/test_declare.py b/tests/test_declare.py index ef1a0fec..5af4f0b7 100644 --- a/tests/test_declare.py +++ b/tests/test_declare.py @@ -358,4 +358,3 @@ class Table_With_Underscores(dj.Manual): schema_any(TableNoUnderscores) with pytest.raises(dj.DataJointError, match="strict CamelCase") as e: schema_any(Table_With_Underscores) - diff --git a/tests/test_utils.py b/tests/test_utils.py index 6697df84..60455132 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -3,7 +3,12 @@ """ from datajoint import DataJointError -from datajoint.utils import from_camel_case, to_camel_case, is_camel_case, contains_non_ascii_char +from datajoint.utils import ( + from_camel_case, + to_camel_case, + is_camel_case, + contains_non_ascii_char, +) import pytest From 850ab4c6d0d698a338c29e86892dccaac0866e2d Mon Sep 17 00:00:00 2001 From: Ethan Ho <53266718+ethho@users.noreply.github.com> Date: Thu, 21 Mar 2024 21:20:40 +0000 Subject: [PATCH 05/11] Add more test cases for is_camel_case --- tests/test_utils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_utils.py b/tests/test_utils.py index 60455132..c167dcc1 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -14,6 +14,9 @@ def test_is_camel_case(): assert is_camel_case("AllGroups") + assert not is_camel_case("All_Groups") + assert not is_camel_case("All_Groups_") + assert not is_camel_case("_AllGroups") assert not is_camel_case("allGroups") assert not is_camel_case("repNames") assert not is_camel_case("10_all") From a88e743ba1da09671d0429f8c6b671dbe2ea9b40 Mon Sep 17 00:00:00 2001 From: Ethan Ho <53266718+ethho@users.noreply.github.com> Date: Thu, 21 Mar 2024 21:20:57 +0000 Subject: [PATCH 06/11] Passing test_is_camel_case --- datajoint/utils.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/datajoint/utils.py b/datajoint/utils.py index b0dcf6a7..04c68825 100644 --- a/datajoint/utils.py +++ b/datajoint/utils.py @@ -76,10 +76,7 @@ def is_camel_case(s): >>> is_camel_case("TableName") # returns True >>> is_camel_case("table_name") # returns False """ - # return re.match(r"^[A-Z][a-z0-9]+(?:[A-Z][a-z0-9]+)*$", s) is not None - return re.match( - r"[A-Z][a-zA-Z0-9]*", s - ) is not None and not contains_non_ascii_char(s) + return re.match(r"^[A-Z][a-z0-9]+(?:[A-Z][a-z0-9]+)*$", s) is not None and not contains_non_ascii_char(s) def to_camel_case(s): From 858265cad6721393b2f56dc9812b86d891c164eb Mon Sep 17 00:00:00 2001 From: Ethan Ho <53266718+ethho@users.noreply.github.com> Date: Thu, 21 Mar 2024 21:21:13 +0000 Subject: [PATCH 07/11] Format with black --- datajoint/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/datajoint/utils.py b/datajoint/utils.py index 04c68825..d729095e 100644 --- a/datajoint/utils.py +++ b/datajoint/utils.py @@ -76,7 +76,9 @@ def is_camel_case(s): >>> is_camel_case("TableName") # returns True >>> is_camel_case("table_name") # returns False """ - return re.match(r"^[A-Z][a-z0-9]+(?:[A-Z][a-z0-9]+)*$", s) is not None and not contains_non_ascii_char(s) + return re.match( + r"^[A-Z][a-z0-9]+(?:[A-Z][a-z0-9]+)*$", s + ) is not None and not contains_non_ascii_char(s) def to_camel_case(s): From 5a8df543f4f4e0aa8d2b7525d6d14a576f56c829 Mon Sep 17 00:00:00 2001 From: Ethan Ho <53266718+ethho@users.noreply.github.com> Date: Fri, 22 Mar 2024 13:20:35 +0000 Subject: [PATCH 08/11] Implement @dimitri-yatsenko suggestion --- datajoint/utils.py | 17 +---------------- tests/test_utils.py | 5 +---- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/datajoint/utils.py b/datajoint/utils.py index d729095e..1aae610d 100644 --- a/datajoint/utils.py +++ b/datajoint/utils.py @@ -53,19 +53,6 @@ def get_master(full_table_name: str) -> str: return match["master"] + "`" if match else "" -def contains_non_ascii_char(s): - """ - Check if a string contains non-ASCII characters. - - :param s: string to check - :returns: True if the string contains any non-ASCII characters, False otherwise - Example: - >>> contains_non_ascii_char("Hello") # returns False - >>> contains_non_ascii_char("HelloΣ") # returns True - """ - return any(ord(c) > 127 for c in s) - - def is_camel_case(s): """ Check if a string is in CamelCase notation. @@ -76,9 +63,7 @@ def is_camel_case(s): >>> is_camel_case("TableName") # returns True >>> is_camel_case("table_name") # returns False """ - return re.match( - r"^[A-Z][a-z0-9]+(?:[A-Z][a-z0-9]+)*$", s - ) is not None and not contains_non_ascii_char(s) + return bool(re.match(r"^[A-Z][A-Za-z0-9]*$", s)) def to_camel_case(s): diff --git a/tests/test_utils.py b/tests/test_utils.py index c167dcc1..88fb355d 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -7,7 +7,6 @@ from_camel_case, to_camel_case, is_camel_case, - contains_non_ascii_char, ) import pytest @@ -23,9 +22,7 @@ def test_is_camel_case(): assert not is_camel_case("hello world") assert not is_camel_case("#baisc_names") assert not is_camel_case("alphaBeta") - non_ascii_class_name = "TestΣ" - assert contains_non_ascii_char(non_ascii_class_name) - assert not is_camel_case(non_ascii_class_name) + assert not is_camel_case("TestΣ") def test_from_camel_case(): From 8755db7ed4cc8b99d4d9ee8ccc18239c8983ba68 Mon Sep 17 00:00:00 2001 From: Ethan Ho <53266718+ethho@users.noreply.github.com> Date: Fri, 22 Mar 2024 13:30:20 +0000 Subject: [PATCH 09/11] Fix failing test_declare test --- tests/test_declare.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_declare.py b/tests/test_declare.py index 5af4f0b7..8939000b 100644 --- a/tests/test_declare.py +++ b/tests/test_declare.py @@ -356,5 +356,7 @@ class Table_With_Underscores(dj.Manual): """ schema_any(TableNoUnderscores) - with pytest.raises(dj.DataJointError, match="strict CamelCase") as e: + with pytest.raises( + dj.DataJointError, match="must be alphanumeric in CamelCase" + ) as e: schema_any(Table_With_Underscores) From 12606f20c76037fddf6c616c9117a1cf4a958073 Mon Sep 17 00:00:00 2001 From: Ethan Ho <53266718+ethho@users.noreply.github.com> Date: Fri, 22 Mar 2024 07:45:46 -0600 Subject: [PATCH 10/11] Update datajoint/table.py Co-authored-by: Dimitri Yatsenko --- datajoint/table.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/datajoint/table.py b/datajoint/table.py index cbc75e99..48f287a9 100644 --- a/datajoint/table.py +++ b/datajoint/table.py @@ -98,9 +98,9 @@ def declare(self, context=None): "e.g. from inside a populate/make call" ) # Enforce strict CamelCase #1150 - if "_" in self.class_name: + if not is_camel_case(self.class_name): raise DataJointError( - "Table with class name `{name}` contains an underscore. ".format( + "Table class name `{name}` is invalid. Please use CamelCase. ".format( name=self.class_name ) + "Classes defining tables should be formatted in strict CamelCase." From f54cd5cb8195274b0699b6f8ba8df62be0416aa0 Mon Sep 17 00:00:00 2001 From: Ethan Ho <53266718+ethho@users.noreply.github.com> Date: Fri, 22 Mar 2024 13:48:19 +0000 Subject: [PATCH 11/11] Fix import error --- datajoint/table.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datajoint/table.py b/datajoint/table.py index 48f287a9..0c571112 100644 --- a/datajoint/table.py +++ b/datajoint/table.py @@ -15,7 +15,7 @@ from .condition import make_condition from .expression import QueryExpression from . import blob -from .utils import user_choice, get_master +from .utils import user_choice, get_master, is_camel_case from .heading import Heading from .errors import ( DuplicateError,