unable to read some svg files

[TuBui]

Function parse_path seems unable to parse some d_strings. The errors I got are "could not convert string to float: L" and "list index out of range".

To reproduce the problem, I enclose two example svg files: https://www.dropbox.com/s/tilysgce9rzo6mj/n01791625_1116-5.svg?dl=0 https://www.dropbox.com/s/15ejoawdrq3lcou/n02439033_628-5.svg?dl=0

Read these files using svg2paths() throws the above error.

[tatarize]

The offending path looks like this:

"M327 468z"

This is the reason that regebro/svg.path added a path Move.

class Move(object):
    """Represents move commands. Does nothing, but is there to handle
    paths that consist of only move commands, which is valid, but pointless.
    """

[tatarize]

The error is that while M sets the initial position, only subsequent commands add actual objects to the pathing command. The call to Z checks the end of the final object within the path.

    def _is_closable(self):
        end = self[-1].end
        for segment in self:
            if segment.start == end:
                return True
        return False

However, since there are no move objects or any pathing yet, it looks for the last element in a 0 element list, which correctly throws an index error.

[TuBui]

Does it mean the svg files are invalid? I can display them properly using a standard image viewer though. Is there a way to check and remove the offending paths in the output of svg2paths() so that the errors could be handled?

[tatarize]

The SVG Specifications allows that type of path to exist. They are somewhat pointless as they are a closed-move-position, but they do not run afoul of the spec. They are absolutely permitted. This is a bug in svgpathtools. Its one that was also, fixed in svg.path from which this is derived. The files are valid, the parsing is valid too, the actual core paths has a bug and does not account for this weird acceptable path and wrongly crashes.

[tatarize]

You would need to modify the offending routine so that it would create a path object without the crash. So in path.py you would need to find the crashing method, which reads:

    def _is_closable(self):
        end = self[-1].end
        for segment in self:
            if segment.start == end:
                return True
        return False

And replace fix that method to include a catch for the case when the path's length is actually 0, and declare that you can close that. The rest should work fine.

    def _is_closable(self):
        if len(self) == 0:
            return True
        end = self[-1].end
        for segment in self:
            if segment.start == end:
                return True
        return False

For fixing the bug, it is actually the case that this will leave that particular path as Path() with no items. When really it explicitly gave a move location and that wasn't preserved. Which is a bit of data lost that doesn't need to be lost.

But, if you want a quick work around, this change will test for the edge case and not run the crashing code if we have nothing in the path.

[tatarize]

This is still a real issue and should be left open @mathandy

[vistuleB]

@tatarize

Which is a bit of data lost that doesn't need to be lost.

I don't see a scenario in which paths that consist only of move commands can influence the rest of an SVG graphic. Is that correct?

[tatarize]

@vistuleB totally. It could matter within the svg path string since somethings depend on previous location, but as it's converted to graphic strings they cannot have any impact on the rendering of the graphics. It's perfectly reasonable to not-crash and simply omit them if the fidelity you want is only the svg graphics. Moves without any drawable statements are not drawn. It's kind of weird that they are permitted.

[javogel]

I'm getting the same error with a different SVG. Are there any workarounds for this issue? or are there any tips on how to automate the removal of those erroneous paths? Thanks!

[youlam]

@javogel I've been maintain/extending my own extension of svgpathtools, with a bunch of goodies the original doesn't have. If you upload your svg file somewhere I can take a look and make sure that my extension can deal with it w/out crashing.

[TuBui]

@javogel While we are waiting for a proper fix (@tatarize , @mathandy do you have a road map for this, perhaps in the next updates?) I do a "quick and dirty" workaround at https://github.com/TuBui/svgpathtools. You install it with:

pip install git+https://github.com/TuBui/svgpathtools#egg=svgpathtools

[tatarize]

My own much more advanced library svgelements (https://github.com/meerk40t/svgelements pip install svgelements) deals with that, and all the ins-and-outs of robust SVG parsing. The path parsing in svgpathtools is not intended to be highly robust. It's intended to be reasonable. So you won't get it to deal with use/defs or css-styles, or css-lengths, or display, or proper shapes beyond paths. You'll hit a nearly endless set of edge cases trying to parse CSS like what if this attribute was omitted, and the spec actually gives you the method to deal with that. That stuff isn't generally the point of svgpathtools which has some of the best math in such libraries around.

Properly fixing this bug would involve switching svgpathtools internal svg.path objects to being similar to version 4.0. Namely it should overtly include real Close and Move. This also permits real subpathing within paths. And gives a genuine 1:1 ratio of commands to path objects.

@javogel I posted above the correction a couple years ago. It's still in the original form, rather than the non-crashing form. The fix I gave still works. Though today I might be more inclined to code that as:

    def _is_closable(self):
        try:
            end = self[-1].end
        except IndexError:
            return True
        for segment in self:
            if segment.start == end:
                return True
        return False

Since it's better to ask for forgiveness than permission. Also note, these are not actually 'erroneous' they are in the spec as legal paths.

[tatarize]

@tatarize , @mathandy do you have a road map for this, perhaps in the next updates?

My roadmap is usually wait for Andy to add whatever my latest thing is and then add another if you want I could make my next pull request for the _is_closable() thing. I tend to leave my PRs quite clean and I'm still waiting on #130. Which is a pretty solid fix.

I'd implement the is_closeable() with the try catch loop to simply prevent the crash and it would exclude the empty subpaths. It works and it's not a radical change.