Skip to content

Commit

Permalink
Restore higher limit for listing top level variables in Python (#2912)
Browse files Browse the repository at this point in the history
Also use the higher limit for updates, as more than 100 new variables could be created in an execution.
Also increase variables returned for refresh case to use MAX_ITEMS
  • Loading branch information
petetronic authored May 1, 2024
1 parent 2d5b6ee commit 6d22cac
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
from positron_ipykernel.positron_ipkernel import PositronIPyKernel
from positron_ipykernel.utils import JsonRecord, not_none
from positron_ipykernel.variables import (
MAX_CHILDREN,
MAX_ITEMS,
VariablesService,
_summarize_children,
_summarize_variable,
Expand Down Expand Up @@ -176,6 +178,82 @@ def test_handle_refresh(shell: PositronShell, variables_comm: DummyComm) -> None
]


def test_list_1000(shell: PositronShell, variables_comm: DummyComm) -> None:
# Create 1000 variables
for j in range(0, 1000, 1):
shell.user_ns["var{}".format(j)] = j

# Request the list of variables
msg = json_rpc_request("list", comm_id="dummy_comm_id")
variables_comm.handle_msg(msg)

# Assert that a message with 1000 variables is sent
result_msg = variables_comm.messages[0]
assert result_msg.get("data").get("result").get("length") == 1000

# Also spot check the first and last variables
variables = result_msg.get("data").get("result").get("variables")
assert variables[0].get("display_name") == "var0"
assert variables[999].get("display_name") == "var999"


def test_update_max_children_plus_one(shell: PositronShell, variables_comm: DummyComm) -> None:
# Create and update more than MAX_CHILDREN variables
n = MAX_CHILDREN + 1
add_value = 500
msg: Any = create_and_update_n_vars(n, add_value, shell, variables_comm)

# Check we received an update message
assert msg.get("data").get("method") == "update"

# Check we did not lose any variables
assigned = msg.get("data").get("params").get("assigned")
assert len(assigned) == n

# Spot check the first and last variables display values
assert assigned[0].get("display_value") == str(add_value)
assert assigned[n - 1].get("display_value") == str(n - 1 + add_value)


def test_update_max_items_plus_one(shell: PositronShell, variables_comm: DummyComm) -> None:
# Create and update more than MAX_ITEMS variables
n = MAX_ITEMS + 1
add_value = 500
msg: Any = create_and_update_n_vars(n, add_value, shell, variables_comm)

# If we exceed MAX_ITEMS, the kernel sends a refresh message instead
assert msg.get("data").get("method") == "refresh"

# Check we did not exceed MAX_ITEMS variables
variables = msg.get("data").get("params").get("variables")
variables_len = len(variables)
assert variables_len == MAX_ITEMS

# Spot check the first and last variables display values
assert variables[0].get("display_value") == str(add_value)
assert variables[variables_len - 1].get("display_value") == str(variables_len - 1 + add_value)


def create_and_update_n_vars(
n: int, add_value: int, shell: PositronShell, variables_comm: DummyComm
) -> Any:
# Create n variables
assign_n = ""
for j in range(0, n, 1):
assign_n += "x{} = {}".format(j, j) + "\n"

shell.run_cell(assign_n)
variables_comm.messages.clear()

# Re-assign the variables to trigger an update message
update_n = ""
for j in range(0, n, 1):
update_n += "x{} = {}".format(j, j + add_value) + "\n"

shell.run_cell(update_n)
return variables_comm.messages[0]


@pytest.mark.asyncio
async def test_handle_clear(
shell: PositronShell,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def _send_update(self, assigned: Mapping[str, Any], removed: Set[str]) -> None:

# Filter out hidden assigned variables
variables = self._get_filtered_vars(assigned)
filtered_assigned = _summarize_children(variables)
filtered_assigned = _summarize_children(variables, MAX_ITEMS)

# Filter out hidden removed variables and encode access keys
hidden = self._get_user_ns_hidden()
Expand Down Expand Up @@ -209,7 +209,7 @@ def send_refresh_event(self) -> None:
}
"""
variables = self._get_filtered_vars()
filtered_variables = _summarize_children(variables)
filtered_variables = _summarize_children(variables, MAX_ITEMS)

msg = RefreshParams(
variables=filtered_variables,
Expand Down Expand Up @@ -423,7 +423,7 @@ def _find_var(self, path: Iterable[str]) -> Tuple[bool, Any]:

def _list_all_vars(self) -> List[Variable]:
variables = self._get_filtered_vars()
return _summarize_children(variables)
return _summarize_children(variables, MAX_ITEMS)

def _send_list(self) -> None:
filtered_variables = self._list_all_vars()
Expand Down Expand Up @@ -731,10 +731,10 @@ def _summarize_variable(key: Any, value: Any) -> Optional[Variable]:
)


def _summarize_children(parent: Any) -> List[Variable]:
def _summarize_children(parent: Any, limit: int = MAX_CHILDREN) -> List[Variable]:
children = []
for i, (key, value) in enumerate(get_inspector(parent).get_items()):
if i > MAX_CHILDREN:
if len(children) >= limit:
break
summary = _summarize_variable(key, value)
if summary is not None:
Expand Down

0 comments on commit 6d22cac

Please sign in to comment.