Svg update

[jmwright]

PRs #1277 and #1325 are both open with SVG export changes, but neither one fixed this issue for me. There was an extra bottom-right margin that I could not remove in the resulting SVG, and I wanted my SVGs to fill the image edge to edge for my use case while still respecting the top/left margins. These changes accomplish that.

Below are before and after examples.

Before:

After:

[codecov]

Codecov Report

Merging #1350 (53eead1) into master (7143ead) will decrease coverage by 0.14%.
The diff coverage is 93.33%.

❗ Current head 53eead1 differs from pull request most recent head 2a8b413. Consider uploading reports for the commit 2a8b413 to get more accurate results

@@            Coverage Diff             @@
##           master    #1350      +/-   ##
==========================================
- Coverage   94.30%   94.17%   -0.14%     
==========================================
  Files          26       26              
  Lines        5584     5594      +10     
  Branches      954      958       +4     
==========================================
+ Hits         5266     5268       +2     
- Misses        187      194       +7     
- Partials      131      132       +1     

Impacted Files	Coverage Δ
cadquery/__init__.py	87.50% <0.00%> (ø)
cadquery/assembly.py	96.61% <ø> (ø)
cadquery/cq.py	92.23% <ø> (ø)
cadquery/occ_impl/exporters/svg.py	96.92% <100.00%> (+0.25%)	⬆️
cadquery/occ_impl/sketch_solver.py	91.19% <100.00%> (ø)

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[adam-urbanczyk]

Thanks, LGTM. I assume that the black border in the "after" picture is some artifact not coming from CQ.

[jmwright]

I assume that the black border in the "after" picture is some artifact not coming from CQ.

Correct. I must have gotten part of the frame of the app when I did the screenshot.

[lorenzncode]

I'd like to propose the following changes:

1. Explicitly choose to recompute width or height.

   • With the current PR the user does not know ahead of time which dimension will be recomputed.

     I get surprised to find either one of width/height changed.

   • It is more straightforward to specify a fixed width or height. The user will likely have one in mind and does not need to specify both since the other is recomputed.

     I can then say I want a width of 200 pixels and the height fits automatically.

2. The current PR handling of margin is inconsistent with fitView=False. I suggest to respect the width or height value given - do not change it according to margin (margin padded to inside of width and height).

Please see these changes here:
lorenzncode@6aab3be

The fitView option was removed. Instead, you can fit by specifying -1 for one of the dimensions.

This results in a width of 200, height is computed:

width = 200
height = -1 

This results in a height of 100, width is computed:

width = -1 
height = 100 

[jmwright]

@lorenzncode I've made those changes (except for the test changes), but I'm not convinced that using -1 for autofit is the best way. Maybe we should allow the user to leave out the other dimension from opts or set it to None.

[adam-urbanczyk]

None sounds more pythonic

[lorenzncode]

Looks good, thanks @jmwright!

[jmwright]

@adam-urbanczyk It looks like AppVeyor is grabbing the OCP 7.7.1 pre-release and it might be breaking the builds.

TypeError: GetColor(): incompatible function arguments. The following argument types are supported:

[adam-urbanczyk]

Should be solved, I moved 7.7.1 to dev

[adam-urbanczyk]

Hm, not quite. Seems that 7.7.1 is available from the conda-forge channel even though the PR is not merged.