camlib.Geometry.bounds() does not recurse

Issue #255 new
Juan Pablo Caram repo owner created an issue

It supports only lists of depth 1. This breaks when merging multiple geometry objects, which creates trees of lists.

Comments (11)

  1. MARCO A QUEZADA

    Hey JP. I had not used flatcam in some time and yesterday I tried using the "Paint All" geometry functionality to fill in some pads but flatcam failed with an exception. The title of the issue caught my eye because the exception that "Paint All" throws is coming from what appears to be a recursive call through a list or tree of geometry objects:

    1.gif

    By the way, single paint of a polygon works without problem. Do you have a description of the structure of the geometry object container as well as polygon objects and how they relate to each other? I'm struggling to decipher it to understand why this recursion is failing.

  2. Juan Pablo Caram reporter

    @codeZonkey , could you please post the full error message? It's only shown for a fraction of a second in the gif.

  3. Juan Pablo Caram reporter

    I think I understand the problem. Someone correct me if I'm wrong. In Python 3, generators should not raise StopIteration. This was left from the Python 2 versions and was not updated correctly. It should be straightforward to fix (basically removing the raise statement, so @codeZonkey, what you did is the correct fix).

    However, this is unrelated to the originally posted issue (which might require some more time to fix).

  4. MARCO A QUEZADA

    @jpcgt, Ok, if there is no issue currently for the bug I found I can create one and submit a commit for it. Are you aware of any other places where I should remove the raise for exceptions? Or is it anywhere the StopIteration is listed? I could go ahead and clean all those up as part of the same commit.

  5. Juan Pablo Caram reporter

    @codeZonkey , could you please first do a quick google search to make sure I'm correct, i.e., that we shouldn't use raise StopIteration in Python3? Then we can merge your patch. Thanks.

  6. MARCO A QUEZADA

    @jpcgt , I am not a proficient python programmer, but I found the following on a different project:

    https://github.com/apertium/apertium-apy/issues/8

    There is a reference in there as to the fact that StopIteration is incompatible with python 3.5 and above. The fix shows that the code was changed to remove the raise StopIteration and replaced them with simple return statements. A reference to official python documentation on the issue is also provided and points to:

    https://docs.python.org/3/whatsnew/3.5.html#pep-479-change-stopiteration-handling-inside-generators

    So I think this validates your understanding. Let me know what you'd want me to do to proceed.

  7. Marius Stanciu

    I think this is already fixed in what is now FlatCAM beta (former FlatCAM 3000). Everything related to Paint is in flatcamTools.ToolPaint()

  8. Juan Pablo Caram reporter

    @marius_stanciu , if you think this is fixed, could you please post a link to the specific commit that fixes it and then close the issue as resolved? Thanks.

  9. Marius Stanciu

    @jpcgt I can't do that because it is part of the Beta fork which, as you know, was created only a few days ago by copying there the FlatCAM 3000 project. There are only a few commits and all of them are newer than the Beta branch creation. They reflect only the work I've done since then fixing things that were reported in the meantime. I could reference the commit in the previous repositories but ... just to find the exact commit is a huge task because are hundreds of commits there ...

    My mention was made because I was under the impression that the beta branch, with some minor changes and improvements will become the next release of FlatCAM therefore I thought there is no need to continue fixing things in the old version as that will be redundant. But perhaps I'm wrong in my perception and if that's the case I stand corrected. Please let me know if that is the case. Thanks!

  10. Juan Pablo Caram reporter

    @marius_stanciu I was just wanting to identify where and how this was fixed. It doesn't matter if it is in beta or not. This will help me make the fix in the master branch, and I can close the issue later. Thanks.

  11. Log in to comment