Python: Safely and efficiently compute variable display values #8545
Conversation
|
E2E Tests 🚀 |
| "pandas.core.indexes.range.RangeIndex": "pandas.RangeIndex", | ||
| "pandas.core.indexes.multi.MultiIndex": "pandas.MultiIndex", | ||
| # Just display Int64Index as pandas.Index, since the former is deprecated since pandas v1.4.0. | ||
| "pandas.core.indexes.numeric.Int64Index": "pandas.Index", |
| @@ -165,10 +174,10 @@ def get_size(self) -> int: | |||
| def has_children(self) -> bool: | |||
| return self.get_length() > 0 | |||
|
|
|||
| def has_child(self, _key: Any) -> bool: | |||
| def has_child(self, key: Any) -> bool: # noqa: ARG002 | |||
There was a problem hiding this comment.
yeah, this makes more sense
|
|
||
| # If the string fits the limit, return it as is. | ||
| # NOTE: We check the actual string length but return the repr which may differ e.g. | ||
| # some characters may be escaped. |
There was a problem hiding this comment.
Is there a reason we don't check len(repr(value))?
There was a problem hiding this comment.
Yep, that is actually related to the bug fixed in this PR, since repr(value) could end up duplicating a huge string. I'll add that to the note
| @@ -225,6 +226,39 @@ def test_change_detection_over_limit(shell: PositronShell, variables_comm: Dummy | |||
| _assert_assigned(shell, big_array, varname, variables_comm, "unevaluated") | |||
|
|
|||
|
|
|||
| def test_change_detection_run_cell_performance(shell: PositronShell, variables_comm) -> None: # noqa: ARG001 need the variables comm to be connected but don't actually use it | |||
There was a problem hiding this comment.
nit: you can get around this noqa by using @pytest.mark.usefixtures("variables_comm") instead!
There was a problem hiding this comment.
Oh that's way nicer, thanks!
| # Also check absolute time - should complete within 1 second. | ||
| assert large_object_ns < 1e9, ( | ||
| f"Simple cell execution took {large_object_ns / 1e6:.3f}ms with large object in namespace" | ||
| ) |
extensions/positron-python/python_files/posit/positron/inspectors.py
Outdated
Show resolved
Hide resolved
| # For items that are the same object as the parent, avoid infinite recursion | ||
| # and represent the item with ellipsis. | ||
| if item is value: | ||
| item_repr, item_truncated = f"{prefix}{ELLIPSIS}{suffix}", True |
There was a problem hiding this comment.
not needed for this PR, but this can get you in a weird state with double ellipses if you end up resizing the variables pane
There was a problem hiding this comment.
Good catch. It does look like an existing issue on main. It isn't technically wrong but I agree it's not ideal. I also feel like I want to double-click it to expand the display value somehow.
…ors.py Co-authored-by: Isabel Zimmerman <[email protected]> Signed-off-by: Wasim Lorgat <[email protected]>
Addresses #8245. The approach is inspired by pydevd which backs the debugger used by most Python IDEs today.
The focus is on safely (i.e. no hangs) and efficiently calculating and sending display values over the wire, not on perfectly formatting those display values, hence the large max character values. The idea is that further formatting would be handled in the UI, although we can change this if needed.
I've mostly left the display formatting of the larger third party objects as is (numpy, pandas, etc). The lower level changes here should make refinements to those easier in future.
Release Notes
New Features
Bug Fixes
QA Notes
See the repro steps in #8245 as well as the new unit tests.
@:variables @:data-explorer @:console