Skip to content

Commit ed8f613

Browse files
committed
Address code review comments
- Simplify check to use idiomatic 'if name_scope_stack:' instead of explicit None and length checks - Remove unnecessary try-except blocks in tests (tests fail automatically on unexpected exceptions) - Use self.assertEqual() directly in multithreaded test for clearer assertions
1 parent 71e2de0 commit ed8f613

File tree

2 files changed

+9
-27
lines changed

2 files changed

+9
-27
lines changed

keras/src/backend/common/name_scope.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ def __exit__(self, *args, **kwargs):
5858
name_scope_stack = global_state.get_global_attribute(
5959
"name_scope_stack"
6060
)
61-
if name_scope_stack is not None and len(name_scope_stack) > 0:
61+
if name_scope_stack:
6262
name_scope_stack.pop()
6363

6464

keras/src/backend/common/name_scope_test.py

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,7 @@ def test_exit_with_none_stack(self):
6262
global_state.set_global_attribute("name_scope_stack", None)
6363

6464
# Exit should not raise an AttributeError
65-
try:
66-
scope.__exit__()
67-
except AttributeError as e:
68-
self.fail(f"__exit__ raised AttributeError: {e}")
65+
scope.__exit__()
6966

7067
# Clean up: reset the stack
7168
global_state.set_global_attribute("name_scope_stack", [])
@@ -84,10 +81,7 @@ def test_exit_with_empty_stack(self):
8481
name_scope_stack.clear()
8582

8683
# Exit should not raise an IndexError
87-
try:
88-
scope.__exit__()
89-
except IndexError as e:
90-
self.fail(f"__exit__ raised IndexError: {e}")
84+
scope.__exit__()
9185

9286
# Verify stack is still empty
9387
name_scope_stack = global_state.get_global_attribute(
@@ -98,22 +92,14 @@ def test_exit_with_empty_stack(self):
9892
def test_multithreaded_name_scope(self):
9993
"""Test name_scope in multithreaded environment."""
10094
results = []
101-
errors = []
10295

10396
def thread_function(thread_id):
104-
try:
105-
# Each thread should have its own name_scope_stack
106-
with name_scope(f"thread_{thread_id}"):
107-
path = current_path()
108-
results.append(path)
109-
# Verify we get the expected path
110-
if path != f"thread_{thread_id}":
111-
errors.append(
112-
f"Thread {thread_id}: Expected "
113-
f"'thread_{thread_id}', got '{path}'"
114-
)
115-
except Exception as e:
116-
errors.append(f"Thread {thread_id}: {type(e).__name__}: {e}")
97+
# Each thread should have its own name_scope_stack
98+
with name_scope(f"thread_{thread_id}"):
99+
path = current_path()
100+
results.append(path)
101+
# Verify we get the expected path
102+
self.assertEqual(path, f"thread_{thread_id}")
117103

118104
# Create and start multiple threads
119105
threads = []
@@ -126,10 +112,6 @@ def thread_function(thread_id):
126112
for thread in threads:
127113
thread.join()
128114

129-
# Check for any errors
130-
if errors:
131-
self.fail(f"Errors in threads: {errors}")
132-
133115
# Verify all threads executed successfully
134116
self.assertEqual(len(results), 5)
135117

0 commit comments

Comments
 (0)