Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs inconsistency about shape.each_point() iterator #547

Closed
dimapu opened this issue Apr 26, 2020 · 6 comments
Closed

Docs inconsistency about shape.each_point() iterator #547

dimapu opened this issue Apr 26, 2020 · 6 comments
Assignees
Labels

Comments

@dimapu
Copy link

dimapu commented Apr 26, 2020

Hi Matthias,

I have found some disagreement in the docs regarding Shape.each_point() iterator.
This page https://www.klayout.de/doc/programming/database_api.html#h2-1056 says:

**Methods applying for polygon and simple polygon shapes**
...
Shape#each_point will deliver every point of the polygon, including that of the holes.
...

The docs for the Shape.each_shape() method on https://www.klayout.de/doc/code/class_Shape.html#method97 say:

This method applies to paths and delivers all points of the path's center line. It will throw an exception for other objects. 

When I try to run the following code on pypi klayout package (v.0.26.4), the statement from the first page is not correct, and exception is raised:

from klayout.dbcore import SimplePolygon, Shapes

# Create a simple polygon
pp = '(1632300,-130000;1631800,-129500;1632050,-129500)'
sp = SimplePolygon.from_s(pp)
print([p for p in sp.each_point()])  # [1632300,-130000, 1631800,-129500, 1632050,-129500]

# Create a shapes container, add a simple polygon there
shapes = Shapes()
shapes.insert(sp)

s = list(shapes.each())[0]
print(type(s))  # <class 'klayout.dbcore.Shape'>
print(s)  # simple_polygon (1632300,-130000;1631800,-129500;1632050,-129500)

if s.is_simple_polygon:
    # This iterator works
    for p in s.simple_polygon.each_point():
        print(p)
    
    # This iterator does not work
    for p in p in s.each_point():  # raises RuntimeError
        print(p)

The error raised is the following: RuntimeError: Internal error: src\db\db\dbShape.cc:159 false was not true in Shape.each_point. The error is an assert statement, so I guess the execution was not supposed to reach there at all...

@klayoutmatthias
Copy link
Collaborator

klayoutmatthias commented Apr 29, 2020

Hi,

well, the error message could be somewhat more comprehensible ... but doc and code are consistent.

The message basically says "I'm expecting a path". A simple polygon isn't a path.

The explanation is: "each_point" only applies to paths. Paths are not polygons, but "wires": a path is a sequence of points and a width. The path connects the points by straight lines with the given width. A simple polygon is a closed loop of points (the vertexes). This is the "hull". A general polygon in addition may have zero to many holes. Holes are also closed loops, but with the opposite orientation of the hull (counterclockwise, while the hull is clockwise).

An overview over the shape types is given here: https://www.klayout.de/doc/programming/geometry_api.html

So for paths: each_point. For simple polygons: each_point_hull. For general polygons: each_point_hull + each_point_hole(n = 0 .. holes-1).

Here is a big fat warning regarding your code: "if s.is_simple_polygon:" is always true! Because "is_simple_polygon" gives you the method object, not the result of the method call. You have to use:

# note the brackets:
if s.is_simple_polygon():
  ..

I hate Python for this!

Matthias

@klayoutmatthias
Copy link
Collaborator

I'll take this ticket as "give me better error messages" ...

@dimapu
Copy link
Author

dimapu commented Apr 29, 2020

Hi Matthias, thanks for looking into this!

Indeed, I should have used brackets in the example. (In my actual code I do :) ).

My remark about the docs was that they say shape.each_point() "will deliver every point" when shape is a polygon, and it does not. It is in the section "Methods applying for polygon and simple polygon shapes" on the page https://www.klayout.de/doc/programming/database_api.html#h2-1056.

Best regards,
Dima.

@klayoutmatthias
Copy link
Collaborator

I see ... that's acutally easy to fix: there is no such method. "each_point" is only there for paths. It would not make much sense to iterate points over hulls and holes if there is no indication which loop the point belongs to. Better use "each_point_hull" and "each_point_hole".

@dimapu
Copy link
Author

dimapu commented Apr 30, 2020

Thanks a lot!

@dimapu
Copy link
Author

dimapu commented Apr 30, 2020

By the way, for simple polygons there is each_point() method: https://www.klayout.de/doc/code/class_SimplePolygon.html#method20 and it works well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants