baddstats / polyclip

R package polyclip: a port of the Clipper library for polygon geometry
19 stars 6 forks source link

Allow polyclip() to work with "open" shapes #13

Closed pmur002 closed 5 years ago

pmur002 commented 5 years ago

Hi

For my 'gridGeometry' package, I created a fork of 'polyclip' that could work with "open" shapes (lines) as well as (closed) polygons.

Would you consider adding this to the real 'polyclip' ?

I have tried to make the patch low-impact (and you can ignore the DESCRIPTION file and the NEWS file). There is just a new 'closed' argument to polyclip(). So you can treat 'A' as either closed or open.

It would be more flexible to allow 'A' to be a combination of closed and open shapes (which I believe Clipper can handle), but that would require much larger and more invasive changes.

Keen to hear what you think

Paul

codecov-io commented 5 years ago

Codecov Report

Merging #13 into master will decrease coverage by 0.05%. The diff coverage is 72.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #13      +/-   ##
==========================================
- Coverage   63.72%   63.66%   -0.06%     
==========================================
  Files           6        6              
  Lines        2994     3003       +9     
==========================================
+ Hits         1908     1912       +4     
- Misses       1086     1091       +5
Impacted Files Coverage Δ
src/init.c 100% <ø> (ø) :arrow_up:
R/clipper.R 87.41% <100%> (+0.08%) :arrow_up:
src/interface.cpp 85.01% <66.66%> (-0.59%) :arrow_down:
src/clipper.h 75% <0%> (-15%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a49b993...64108b1. Read the comment docs.

pmur002 commented 5 years ago

I see from the nice code coverage report that I should probably have added an example to the documentation (?). Happy to do that if required.

baddstats commented 5 years ago

Excellent. Thank you! I wlil merge this later today. A

pmur002 commented 5 years ago

Awesome. Thanks!