Fix matplot pt-unit stripping inflating whole-number dims 100x (#1654)

[Maya-Mohamed]

Description

Fixes the pt-unit stripping logic in matplot() when resizable=True.
The cleanup line used .replace("pt", "00"), which substitutes the literal
pt for 00 instead of removing it. For whole-number SVG dimensions like
"300pt" this produced "30000" — a silent 100x oversized pane. Decimal
values like "345.6pt" only worked by coincidence ("345.600"). Changed
both the height and width lines to .replace("pt", "") so the unit is
stripped correctly regardless of decimals.

Motivation and Context

Fixes #1654. Panes rendered ~100x too large with no error or warning
whenever a matplotlib plot's SVG dimensions came out as whole points.

How Has This Been Tested?

• Reproduced the bug locally with a fake plot emitting height="300pt" width="400pt": before the fix matplot(..., opts={"resizable": True})
  computed height=42000.0, width=54000.0; after the fix it computes the
  correct height=420.0, width=540.0 (1.4300, 1.35400).
• Confirmed the decimal path is unchanged ("345.6pt" still correct).
• Ran the existing suite: python -m unittest py.tests.test_plots -v
  — all 42 tests pass.
• Ran black py — no formatting changes.

Types of changes

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• I adapted the version number under py/visdom/VERSION according to Semantic Versioning
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

Summary by Sourcery

Fix incorrect handling of matplotlib SVG pt units in matplot() resizable plots to prevent 100x oversized panes and bump the visdom package version accordingly.

Bug Fixes:

• Correct pt-unit stripping in matplot() when resizable to use empty replacement instead of "00", ensuring SVG height and width are interpreted at the correct scale for both integer and decimal values.

Chores:

• Update py/visdom/VERSION to reflect the bugfix release.

[sourcery-ai]

🧙 Sourcery has finished reviewing your pull request!

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• Since the regex already captures the numeric part (([0-9\.]* )pt), you could simplify the height/width handling by always using group(1) and removing the replace("pt", "") calls entirely to avoid dual code paths.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Since the regex already captures the numeric part (`([0-9\.]* )pt`), you could simplify the height/width handling by always using `group(1)` and removing the `replace("pt", "")` calls entirely to avoid dual code paths.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[Maya-Mohamed]

Quick note on testing: the matplot() resizable path currently has no
test coverage, so this fix isn't guarded against regressions.

I'm happy to add a small regression test if useful. My plan would be a
single unittest.TestCase in py/tests/test_plots.py (following the
existing send=False + patch pattern used by TestLine/TestScatter)
that:

• feeds matplot() a tiny fake plot whose savefig emits an SVG with
  height="300pt" width="400pt",
• patches self.viz.svg to capture the computed opts,
• asserts opts["height"] == 420.0 and opts["width"] == 540.0
  (i.e. 1.4*300 / 1.35*400) instead of the pre-fix 42000.0/54000.0,
• plus one decimal case ("345.6pt") to confirm that path still works.

It'd be guarded with @unittest.skipUnless(...) so it's skipped where
bs4/lxml aren't installed, and it only needs math (already a stdlib
import in visdom/__init__.py).

Would you like this in this PR, or would you prefer to keep the change
minimal as-is?

[Manik-Khajuria-5]

@rajnisht7 @Jayantparashar10 @tonypzy @vedansh-5 Please review this PR

[Saksham-Sirohi]

Copilot flagged the following:

• Fix looks correct ("10pt" no longer becomes "1000").
• Confirm whether the `0.

[Maya-Mohamed]

Thanks for the review @Saksham-Sirohi!

Addressed both points:

1. Regression test — added a TestMatplotResizable case in
   py/tests/test_plots.py using a mocked SVG with height="432pt"
   (and width="640pt"). It routes through the resizable=True
   BeautifulSoup path and asserts opts["height"] == 1.4 * ceil(432) == 604.8 instead of the pre-fix 60480.0. I also added a decimal
   case (345.6pt) to confirm the values that worked by coincidence
   before still work. The test is guarded with
   @unittest.skipUnless(visdom.BS4_AVAILABLE, ...) so it skips
   cleanly where bs4/lxml aren't installed.

2. Version bump — good call, I've reverted it. Looking at the
   history, py/visdom/VERSION is only changed in dedicated release
   commits by maintainers, never bundled with a fix, so I've kept this
   PR limited to the fix + test and left the release bump to you all.

[Manik-Khajuria-5]

@Maya-Mohamed Please resolve the conflicts

[Maya-Mohamed]

@Maya-Mohamed Please resolve the conflicts

Done.

[Manik-Khajuria-5]

@Maya-Mohamed Code changes LGTM! But please go through below mentioned test and see whether it is compatible with current codebase or some changes in requirement.txt is required

[Manik-Khajuria-5]

@Maya-Mohamed Please make it compatible with pytest .

[Maya-Mohamed]

@Manik-Khajuria-5 Added beautifulsoup4 and lxml to test-requirements.txt. CI installs from
that file before running pytest, so the TestMatplotResizable tests now
actually run under pytest instead of being skipped (the skipUnless guard
was skipping them because bs4/lxml weren't installed in CI). Confirmed
locally: pytest py/tests/test_plots.py::TestMatplotResizable -v shows
2 passed. Let me know if you'd instead prefer a pytest-native
@pytest.mark.skipif — happy to switch.

[Manik-Khajuria-5]

@Maya-Mohamed Thanks for the fix and for confirming it locally! Since we're integrating pytest, I'd lean towards using @pytest.mark.skipif

[Maya-Mohamed]

@Maya-Mohamed Thanks for the fix and for confirming it locally! Since we're integrating pytest, I'd lean towards using @pytest.mark.skipif

thanks for the review! I made the switch to @pytest.mark.skipif.