[u7k4rs6]

Summary

display_or_print_df styles the dataframe with df.style.applymap(...). Styler.applymap was deprecated in pandas 2.1 and removed in pandas 3.0, so with pandas 3 that line raises an AttributeError. The bare except around it swallows the error and falls back to the plain-text tabulate print, so in a notebook print_data_tabular() silently stops showing the styled table, no error anywhere.

The fix is just applymap -> map. Styler.map exists since pandas 2.1 and the repo already needs pandas>=2.2.2, so nothing else changes.

Test plan

Added test_display_or_print_df_in_ipython in devtools/inspector/tests/inspector_utils_test.py. It fakes the IPython modules, turns FutureWarning into an error (so it also catches the deprecated call on pandas 2.x), and checks the styled table goes to display() and nothing is printed. It fails on main and passes with the fix on pandas 3.0.6, and the rest of inspector_utils_test.py passes (93 tests). ufmt and flake8 are clean with the versions in requirements-lintrunner.txt.

cc @Gasoonjia @nil-is-all

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/23690

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (4 Unrelated Failures)

As of commit 6c65de0 with merge base b1beef3 ():

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

• pull / nxp-mcuxpresso-test / linux-job (gh) (trunk failure)
  RuntimeError: Command bash /__w/_temp/exec_script failed with exit code 1
• pull / unittest / windows / windows-job (gh) (trunk failure)
  .ci/scripts/tests/test_muse_glimmer_batching_workload.py::MuseGlimmerBatchingWorkloadTest::test_workload_without_a_model
• pull / unittest-editable / windows / windows-job (gh) (trunk failure)
  .ci/scripts/tests/test_muse_glimmer_batching_workload.py::MuseGlimmerBatchingWorkloadTest::test_workload_without_a_model
• pull / unittest-nxp-neutron / linux-job (gh) (trunk failure)
  RuntimeError: Command bash /__w/_temp/exec_script failed with exit code 1

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[u7k4rs6]

@pytorchbot label "release notes: devtools"

[nil-is-all]

@u7k4rs6 could you update branch and ping for review?

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[u7k4rs6]

thanks @nil-is-all, updated the branch. @Gasoonjia could you take a look when you get a chance? small one, applymap -> map for pandas 3.

[nil-is-all]

@claude review this PR

[claude]

Claude finished @nil-is-all's task in 2m 26s —— View job

---

The fix is correct and fine to merge; the new test has a few minor robustness gaps, none of them blocking.

Detailed review

PR Review: #23690

Summary

This PR switches display_or_print_df from Styler.applymap to Styler.map, so the styled table renders again in notebooks on pandas 3, and adds a regression test for it. The source change is correct: the repo pins pandas>=2.2.2 (setup.py:1164), and no other applymap/Styler calls remain in the repo. Every remaining finding is minor and concerns the test or the error handling around this code.

Code Quality

• devtools/inspector/_inspector_utils.py:390: the bare except: is what hid this bug. It still catches every error in the IPython branch: a removed pandas API, a missing jinja2 (df.style needs it), or a broken style function. All of them fall back silently to tabulate. Catching only ImportError, and replacing the self-raised Exception at line 387 with a plain branch to the print fallback, would let the next break like this one surface. This is optional because it is existing code, but it is the root cause. Fix this →

Infrastructure

• devtools/inspector/tests/targets.bzl:40-52: the test now does import pandas as pd directly, but inspector_utils_test does not list fbsource//third-party/pypi/pandas:pandas. It only gets pandas indirectly through //executorch/devtools:lib. df.style also needs jinja2 at runtime, and no Buck target in the repo declares it. In OSS, jinja2 arrives with torch. Internally, if the third-party pandas target does not bring it in, the bare except swallows the ImportError and the test fails with only 0 != 1. Low confidence: I could not check the internal build, so it is worth confirming that the internal pandas is ≥2.1 and that jinja2 is available.

Testing

• devtools/inspector/tests/inspector_utils_test.py:305: Styler.map does nothing until the table is rendered, and the fake display is just list.append. So style_text_size never runs, and the test never checks that a Styler was displayed. A broken style function would still pass. Asserting isinstance(displayed[0], pd.io.formats.style.Styler) and that "font-size: 12px" appears in displayed[0].to_html() would exercise the real path and also prove jinja2 is present.
• devtools/inspector/tests/inspector_utils_test.py:301-303: simplefilter("error", FutureWarning) turns any FutureWarning raised inside the call into an error, including ones from building the Styler or importing jinja2. The bare except then swallows it, so an unrelated deprecation in a future pandas or jinja2 release fails this test with only 0 != 1. The filter only matters on pandas 2.1–2.x; on pandas 3 the old call raises AttributeError anyway. Narrowing it, for example warnings.filterwarnings("error", message=".*applymap.*", category=FutureWarning), would avoid this. The comment on line 302 also describes applymap, which the code under test no longer uses.

Recommendation

Approve

Nothing blocks the merge. The test improvements and the deps check are suggestions. The bare-except cleanup can be a follow-up.

Notes on this review: the reviewers could not run the test because Python execution was not permitted, so all findings come from reading the code. origin/main had no merge base in the shallow checkout, so the diff was taken against the merge base b1beef31.

[claude]

Claude finished @nil-is-all's task in 1m 55s —— View job

---

Summary: The applymap → map fix is correct and complete, and the new test catches the regression on both pandas 2.x and 3.x. I found only minor test-robustness nits, and I recommend approving.

Detailed review

PR Review: #23690

Summary

The PR switches display_or_print_df from Styler.applymap to Styler.map so the styled table works again with pandas 3, and adds an IPython-path test. No blocking issues. I checked the pins: setup.py:1164 and .ci/docker/requirements-ci.txt:11 require pandas>=2.2.2, and pyproject.toml requires Python ≥3.11, so Styler.map is always there. No other applymap calls are left in the repo.

Testing

• devtools/inspector/tests/inspector_utils_test.py:303: simplefilter("error", FutureWarning) applies to the whole display_or_print_df call, including Styler/jinja2 internals. A future, unrelated pandas or jinja2 deprecation would raise inside the call. The bare except: would swallow it and fall back to print, and the test would fail with len(displayed) == 0 and no hint of the cause. A missing jinja2 (df.style needs it) would fail the same way. Either narrow the filter, e.g. warnings.filterwarnings("error", message=".*applymap.*", category=FutureWarning), or pass out.getvalue() as the assertion message so failures are easier to diagnose.
• devtools/inspector/tests/inspector_utils_test.py:305: the test only checks len(displayed) == 1. Checking that the displayed object is a pandas.io.formats.style.Styler would also catch a regression that displays the raw df.
• devtools/inspector/tests/inspector_utils_test.py:290-291: the leading comment repeats the test name and assertions, which goes against the minimal-comment rule in CLAUDE.md. The deprecation comment at line 302 is useful and can stay.

Code Quality

• devtools/inspector/_inspector_utils.py:390 (existed before this PR, optional follow-up): the bare except: is the reason this broke silently. It also catches KeyboardInterrupt/SystemExit. Narrowing it to except ImportError plus the explicit non-IPython case, or at least to except Exception, would make future styling breakages visible instead of silently falling back.

Recommendation

Approve: nothing above blocks the merge. I didn't run the new test here because the sandbox wouldn't allow running Python, so this review is based on reading the code.

[nil-is-all]

@u7k4rs6 I'm running some AI-assisted reviews on your CUDA & devtools PRs. @Gasoonjia will be back on Nov 2 and will be able to provide additional reviews & approvals