First pull request of my life, pls help

[vistuleB]

Hi, I've never contributed to an open source project before, I hardly know how to use git. (Mathematician here.)

Could someone guide me through my first pull request?

The modifications I've made (small to big)

-- correct bug in path_encloses_pt [current bug: double-counts intersections at segment endpoints]

-- add a "multisplit" functionality on top of "split" to paths

-- add a feature for approximating arc segments by a sequence of cubic bezier curves... so now any path can be turned into a purely bezier path... the user can control the accuracy of the approximation, as well as well as the "safety" (number of points measured along the curve to ensure said accuracy is respected)... (The cubic beziers I used are those described in the paper "Drawing an Elliptical Arc using Polylines, Quadratic or Cubic Bezier Curves" by L. Maisonobe.)

-- (the above could also be used for measuring area of paths with arc segments, though I didn't make the change since I don't need area myself)

-- path offsets: offset a path by some amount; exact offset is not possible for bezier curves (or arcs), so the user gets to choose an accuracy, and bezier curves are recursively (and adaptively) subdivided into smaller bezier curves (of same degree) until the desired accuracy is respected everywhere; accurately produces cusps, etc, for concave offsets; requires the "arc to bezier" transformation above for paths with elliptical arcs (i.e., elliptical arcs are first transformed into a sequence of cubic beziers, after which these are offset)

-- joins: afore-mentioned path offset accommodates the standard SVG segment joins: miter, bevel, and round; miter-limit also implemented

-- path stroke: two-sided path offset with added line caps and line cap options 'butt', 'square' and 'round'---basically, now one can implement a "stroke to path" operation, as found in popular software like inkscape in 1 command, without resorting to approximation by polylines, and with all the standard line cap/segment join SVG options

-- also works with discontinuous and/or closed paths and/or mixtures thereof :)

-- fun fact: the above offset and stroke operations are robust enough to be used with negative offsets / stroke widths... in such a case the endcaps of the path take an inverted shape, as expected.

I'd like to contribute all this to the repo, but will need a little bit of guidance... thanks...

[mathandy]

Hi @jpsteinb, I'm excited to see your additions and improvements. Here's a simple way to make a pull-request

1. Create a fork by going to https://github.com/mathandy/svgpathtools and clicking the "Fork" button in the upper right.

2. Clone your fork by opening a terminal and using the command git clone https://github.com/jpsteinb/svgpathtools

3. Go into the directory created, which is likely called "svgpathtools-master" and replace the any files you modified with your own versions.

4. Make a new git checkpoint by executing (from inside the "svgpathtools-master" directory) the command git commit -am "added some sweet improvements" <-- not that that's a professional/useful commit message, but it's your first time, so no sweat ;)

5. Push that checkpoint to your online github fork by simply executing git push

6. Go online to your GitHub repo and press the "New pull request" button (next to the branch button around the top left)

If you're curious about any of these steps or want some images to guide you, here might be a source.

[vistuleB]

Thank you, that's just what I needed.

I've been taking some time because in the meanwhile I've done a complete refactor of path.py to accommodate the SVG spec. (Now paths containing several internal Z's and/or several internal "in-place movetos" are correctly handled.) I'm going to work/debug with this refactored version a few more days before submitting---thanks. (If you have some test suite I should be aware of you can let me know.)

[mathandy]

@jpsteinb please keep in mind a lot of people use this code and changes in behaviors/names will likely break scripts that use svgpathtools -- so new features are great, and many aspects of svgpathtools could be implemented in better ways, the single most important factor in determining whether to accept a pull-request is whether is has the potential to break other software that uses svgpathtools. On a related note -- and to answer your question -- yes, we do use the unittest test suite. Please see here for examples. Also you might find some of our official contributor guidelines useful.

[vistuleB]

Hi Andy,

Is the pull request hopeless? If so, do you advise that I open my own fork? (Thanks.)

[SebKuzminsky]

I'm not Andy and I don't mean to speak for him, but I've contributed to svgpathtools and I help maintain several other open source projects so I have some experience sending and receiving PRs.

The best PRs (ie, the ones that are easiest for the upstream maintainers to accept, or to give useful feedback on) are small and focused on a single change. Good PRs like this may have a single commit or a small handful of related commits that together implement the change you're proposing. The PR may fix a bug or add a feature (along with documenting and testing the new feature). When creating a PR, describe the rationale for the change you're proposing, and describe any impact it will have on users, including backwards compatibility and any other risks to existing functionality.

If you have several changes you want to submit, the best way to do so is to take the simplest, most-likely-to-be-accepted change and submit just that as a stand-alone PR. Large, sprawling PRs that change many things at once are very difficult to accept.

Forking a project is always an option, of course, but it has the sad downsides of dividing the effort applied to the main line, and reducing the user base for one or both of the projects (main line and the fork). It's more work to merge your changes into mainline (for you and for the mainline maintainers), but it serves the community better, and it will increase the userbase that will benefit from your changes.

[vistuleB]

OK,

I hope Andy will comment at some point. The pull request in question does some from the ground up refactoring. I don't think it can be bundled into a sequence of small separate changes. (By the way the pull request has already been made, please take a look and let me know if something about it is unclear.)

I was mainly interested if Andy could tell me if it's worth it for me to rewrite the test suite for this pull request, or do something else that might increase the pull request's chances of success.

Thanks.