mathandy / svgpathtools

A collection of tools for manipulating and analyzing SVG Path objects and Bezier curves.
MIT License
557 stars 142 forks source link

bpoints2bezier() does not always return a value #204

Closed vasarhelyi closed 1 year ago

vasarhelyi commented 1 year ago

This function in path.py might return None if order is not in [1, 2, 3]. Would be better to raise a ValueError or return something explicit instead.

mathandy commented 1 year ago

It does -- the assert len(bpoints) in {2, 3, 4} raises an error in exactly that case unless I'm missing something.

def bpoints2bezier(bpoints):
    """Converts a list of length 2, 3, or 4 to a CubicBezier, QuadraticBezier,
    or Line object, respectively.
    See also: poly2bez."""
    order = len(bpoints) - 1
    if order == 3:
        return CubicBezier(*bpoints)
    elif order == 2:
        return QuadraticBezier(*bpoints)
    elif order == 1:
        return Line(*bpoints)
    else:
        assert len(bpoints) in {2, 3, 4}

I'm closing this issue, but please re-open if I've missed something.

vasarhelyi commented 1 year ago

OK sorry, you are right, good as it is...

vasarhelyi commented 1 year ago

Ok, sorry, I remember now why I wrote this. The VSCode editor and its code checker is complaining abut this function as there is no explicit raise ValueErrror(), only an assert, so it might seem for a non-intelligent type checker that there is a branch in which None might be returned after the assert (if it does not fail). In this sense an explicit ValueError might be slightly better to state that there is no return point in that branch of the if else in any case.

ntamas commented 1 year ago

Note that all assert statements are ignored / removed by Python when it is run with optimizations (i.e. python -O). I think assertions are also removed when compiling Python bytecode with optimization level 2, so in general you should not use assert if you want this error to show up for all users no matter how their environment is set up.