You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Honor baseunit when deriving SVG dimensions from viewBox - #255
When viewbox supplies the SVG dimensions, disvg currently ignores baseunit. A viewBox of (0, 0, 2.5, 3.75) with baseunit='cm' produces unitless width='2.5' and height='3.75', rather than the requested centimeter dimensions.
Append non-pixel unit strings to inferred dimensions. Keep default/explicit pixel output and legacy non-string handling unchanged; explicit dimensions and svg_attributes width/height still take precedence.
The native regressions cover tuple/string viewboxes, cm/mm/in/% units, pixel/empty/None controls, explicit dimensions, partial SVG attribute overrides, Drawing serialization and separate files written through disvg and wsvg. They also check preserved viewBox/path data and automatic sizing.
Validation from the repository root on Linux, Python 3.12:
python -m unittest discover test: 115 tests, OK.
Restoring the original export module produces 13 expected assertion failures in the new tests.
python -m build --no-isolation: sdist and wheel built.
Other OS/Python versions were not run locally. The existing slow area test remains disabled by upstream's RUN_SLOW_TESTS setting.
PR #252 adds nearby type annotations but leaves this dimension inference unchanged; a mechanical rebase may be needed.
Summary by CodeRabbit
New Features
SVGs with a supplied viewBox and no explicit dimensions now use the selected baseunit for inferred display dimensions when it is a string other than px. ViewBox coordinates remain unitless.
Explicit dimensions and SVG attributes continue to take precedence over inferred dimensions.
Documentation
Clarified how baseunit, explicit dimensions, and SVG attributes affect inferred display dimensions.
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 11d5c97c-4f0e-4fd0-8e3a-c2a1fdf8689c
📥 Commits
Reviewing files that changed from the base of the PR and between 15b500e and 79f79c5.
📒 Files selected for processing: 2
svgpathtools/paths2svg.py
test/test_paths2svg.py
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
📝 Walkthrough
When viewbox is provided without dimensions, inferred SVG dimensions now append baseunit when it is a string other than 'px'. Tests cover supported base units, saved output, and precedence for explicit dimensions and SVG attributes.
Changes: SVG dimensions and base units
Layer / File(s)
Summary
Infer dimensions with base units svgpathtools/paths2svg.py, test/test_paths2svg.py
When viewbox is provided without dimensions, inferred width and height now receive the baseunit suffix for string values other than 'px'. The documentation describes this behavior and the precedence of explicit dimensions and SVG attributes. Tests cover inferred dimensions, saved output from disvg and wsvg, and those precedence rules.
The change appears ready to merge after normal checks; no unresolved issue was identified with viewBox-derived dimensions or their overrides.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files.
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: applying baseunit when deriving SVG dimensions from viewBox.
Linked Issues check
Issue [#215] requires disvg to include baseunit when it derives SVG dimensions from viewbox. In svgpathtools/paths2svg.py, the viewbox and omitted dimensions path appends a string `baseuni…
Out of Scope Changes check
The changes are limited to the [#215] dimension inference behavior, its public documentation, and regression tests. The tests cover disvg, wsvg, Drawing serialization, viewBox and path preservat…
Full details: Linked Issues check
Explanation
Issue [#215] requires disvg to include baseunit when it derives SVG dimensions from viewbox. In svgpathtools/paths2svg.py, the viewbox and omitted dimensions path appends a string baseunit other than 'px' to both inferred dimensions. Pixel, empty, and None controls retain unitless output. Explicit dimensions and svg_attributes width/height retain precedence. The added tests cover tuple and string viewboxes, requested units, file output, and precedence behavior.
Full details: Out of Scope Changes check
Explanation
The changes are limited to the [#215] dimension inference behavior, its public documentation, and regression tests. The tests cover disvg, wsvg, Drawing serialization, viewBox and path preservation, and precedence cases that support the objective. No unrelated production behavior is identified.
✨ Finishing Touches
🧪 Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #215.
When
viewboxsupplies the SVG dimensions,disvgcurrently ignoresbaseunit. A viewBox of(0, 0, 2.5, 3.75)withbaseunit='cm'produces unitlesswidth='2.5'andheight='3.75', rather than the requested centimeter dimensions.Append non-pixel unit strings to inferred dimensions. Keep default/explicit pixel output and legacy non-string handling unchanged; explicit
dimensionsandsvg_attributeswidth/height still take precedence.The native regressions cover tuple/string viewboxes, cm/mm/in/% units, pixel/empty/None controls, explicit dimensions, partial SVG attribute overrides, Drawing serialization and separate files written through
disvgandwsvg. They also check preserved viewBox/path data and automatic sizing.Validation from the repository root on Linux, Python 3.12:
python -m unittest discover test: 115 tests, OK.python -m build --no-isolation: sdist and wheel built.Other OS/Python versions were not run locally. The existing slow area test remains disabled by upstream's
RUN_SLOW_TESTSsetting.PR #252 adds nearby type annotations but leaves this dimension inference unchanged; a mechanical rebase may be needed.
Summary by CodeRabbit
baseunitfor inferred display dimensions when it is a string other thanpx. ViewBox coordinates remain unitless.baseunit, explicit dimensions, and SVG attributes affect inferred display dimensions.