skrawcz commented on code in PR #1485: URL: https://github.com/apache/hamilton/pull/1485#discussion_r2836536707
########## tests/test_graph.py: ########## @@ -52,6 +52,40 @@ import tests.resources.typing_vs_not_typing [email protected]( + ("child_name", "parent_name", "expected"), + [ + ("foo", "foo", True), # same module + ("foo.bar", "foo", True), # direct child + ("foo.bar.baz", "foo", True), # nested child + ("foo.bar.baz", "foo.bar", True), # nested child of subpackage + ("foobar", "foo", False), # not a submodule, just a prefix without dot separator + ("hamilton.function_modifiers", "modifiers", False), # substring match, not a submodule + ("hamilton.function_modifiers.dependencies", "modifiers", False), # substring deeper + ("x.foo.y", "foo", False), # parent name in the middle, not a prefix + ("bar", "foo", False), # completely unrelated + ], + ids=[ + "same_module", + "direct_child", + "nested_child", + "nested_child_of_subpackage", + "prefix_without_dot", + "substring_not_submodule", + "substring_deeper", + "parent_in_middle", + "unrelated", + ], +) +def test_is_submodule(child_name, parent_name, expected): + """Tests that is_submodule correctly checks module hierarchy using prefix matching.""" + from types import ModuleType Review Comment: top level import please. ########## tests/test_graph.py: ########## @@ -64,6 +98,44 @@ def test_find_functions(): assert actual == expected +def test_find_functions_excludes_imports_with_substring_module_name(): + """Regression test: imported functions should not be included when the user module's + name is a substring of the imported function's module path. + + Previously, is_submodule used `parent.__name__ in child.__name__` (substring match), + which caused e.g. a module named 'modifiers' to pull in functions from + 'hamilton.function_modifiers'. + """ + import sys + from types import ModuleType + + from hamilton.function_modifiers import source, value Review Comment: top level imports please -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
