Skip to content

Commit ff7d6e5

Browse files
committed
reflect PR feedback
1 parent eff35d9 commit ff7d6e5

File tree

6 files changed

+74
-10
lines changed

6 files changed

+74
-10
lines changed

src/designer_plugin/d3sdk/ast_utils.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -535,8 +535,12 @@ def find_imports_for_function(func: Callable[..., Any]) -> list[PackageInfo]:
535535
if not _is_supported_module(alias.name):
536536
continue
537537

538-
# The name used in code is the alias if present, otherwise the module name
539-
effective_name = alias.asname if alias.asname else alias.name
538+
# The name used in code is the alias if present, otherwise the top-level
539+
# package name (e.g. "import logging.handlers" binds "logging", not
540+
# "logging.handlers").
541+
effective_name = (
542+
alias.asname if alias.asname else alias.name.split(".")[0]
543+
)
540544
if effective_name in used_names:
541545
packages.append(
542546
PackageInfo(

src/designer_plugin/d3sdk/client.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ def create_d3_plugin_method_wrapper(
8585
2. Serializes the arguments using repr()
8686
3. Builds a script string in the form: "return plugin.{method_name}({args})"
8787
4. Creates a PluginPayload with the script and module information
88-
5. Sends it to Designer via d3_api_plugin or d3_api_aplugin
88+
5. Sends it to Designer via d3_api_execute or d3_api_aexecute
8989
6. Returns the result from the remote execution
9090
9191
Args:

src/designer_plugin/d3sdk/function.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,15 +263,11 @@ def __init__(self, module_name: str, func: Callable[P, T]):
263263

264264
super().__init__(func)
265265

266-
# Auto-register packages used by this function
267-
D3Function._available_packages[module_name].update(
268-
pkg.to_import_statement() for pkg in self._function_info.packages
269-
)
270-
271266
# Update the function in case the function was updated in the same session.
272267
# For example, jupyter notebook server can be running, but function signature can
273268
# change constantly.
274-
if self in D3Function._available_d3functions[module_name]:
269+
is_replacement = self in D3Function._available_d3functions[module_name]
270+
if is_replacement:
275271
logger.debug(
276272
"Function '%s' in module '%s' is being replaced.",
277273
self.name,
@@ -280,6 +276,19 @@ def __init__(self, module_name: str, func: Callable[P, T]):
280276
D3Function._available_d3functions[module_name].discard(self)
281277
D3Function._available_d3functions[module_name].add(self)
282278

279+
if is_replacement:
280+
# Full rebuild needed to evict stale imports from the replaced function.
281+
D3Function._available_packages[module_name] = {
282+
pkg.to_import_statement()
283+
for f in D3Function._available_d3functions[module_name]
284+
for pkg in f._function_info.packages
285+
}
286+
else:
287+
# New function: incrementally add its packages. No stale imports to remove.
288+
D3Function._available_packages[module_name].update(
289+
pkg.to_import_statement() for pkg in self._function_info.packages
290+
)
291+
283292
def __eq__(self, other: object) -> bool:
284293
"""Check equality based on function name for unique registration.
285294

src/designer_plugin/d3sdk/session.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ def __init__(self, hostname: str, port: int, context_modules: set[str]) -> None:
3535
Args:
3636
hostname: The hostname of the Designer instance.
3737
port: The port number of the Designer instance.
38-
context_modules: List of module names to register when entering session context.
38+
context_modules: Set of module names to register when entering session context.
3939
"""
4040
self.hostname: str = hostname
4141
self.port: int = port

tests/test_ast_utils.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import ast
77
import inspect
8+
import logging.handlers
89
import textwrap
910
import types
1011
from os.path import join as path_join
@@ -1208,6 +1209,16 @@ def dummy():
12081209
packages = find_imports_for_function(dummy)
12091210
assert packages == []
12101211

1212+
def test_dotted_import_effective_name(self):
1213+
"""import logging.handlers binds 'logging' — should match usage of logging.handlers."""
1214+
1215+
def uses_logging_handlers():
1216+
return logging.handlers.RotatingFileHandler("/tmp/x")
1217+
1218+
packages = find_imports_for_function(uses_logging_handlers)
1219+
statements = [p.to_import_statement() for p in packages]
1220+
assert "import logging.handlers" in statements
1221+
12111222

12121223
if __name__ == "__main__":
12131224
pytest.main([__file__, "-v"])

tests/test_core.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,4 +514,44 @@ def func_using_logging():
514514
assert payload is not None
515515
assert "import logging" in payload.contents
516516

517+
def test_new_functions_accumulate_packages(self):
518+
"""Adding a second function should add its packages without losing the first's."""
519+
module = "test_accumulate_pkg_module"
520+
D3Function._available_d3functions[module].clear()
521+
D3Function._available_packages[module].clear()
522+
523+
@d3function(module)
524+
def func_a():
525+
return logging.getLogger("a")
526+
527+
assert "import logging" in D3Function._available_packages[module]
528+
529+
@d3function(module)
530+
def func_b():
531+
import os
532+
return os.getcwd()
533+
534+
# Both packages must be present after adding func_b
535+
assert "import logging" in D3Function._available_packages[module]
536+
assert "import os" in D3Function._available_packages[module]
537+
538+
def test_replacement_removes_stale_packages(self):
539+
"""Replacing a function with one that uses fewer imports should evict stale packages."""
540+
module = "test_stale_pkg_module"
541+
D3Function._available_d3functions[module].clear()
542+
D3Function._available_packages[module].clear()
543+
544+
@d3function(module)
545+
def my_func(): # uses logging
546+
return logging.getLogger("x")
547+
548+
assert "import logging" in D3Function._available_packages[module]
549+
550+
@d3function(module)
551+
def my_func() -> int: # noqa: F811 # no longer uses logging
552+
return 42
553+
554+
# Stale import from the old version must be gone
555+
assert "import logging" not in D3Function._available_packages[module]
556+
517557

0 commit comments

Comments
 (0)