-
Notifications
You must be signed in to change notification settings - Fork 20
Add support of holes to contours generation #335
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
Add support of holes to contours generation #335
Conversation
ea673c9
to
8094ff5
Compare
self.shape = shape | ||
self.label = label | ||
self.probability = probability | ||
self.excluded_shapes = excluded_shapes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For self.shape
and self.excluded_shapes
, I would suggest to force the type to be np.ndarray
or list[tuple[int, int]]
, but not both. Otherwise clients who consume contour.shape
have to support both types, which isn't great.
Note: this is different from the class constructor parameter, which is instead an input interface. There, supporting more types is helpful for the clients. This principle is somehow formulated in the Postel law.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally prefer garbage in garbage out principle: the system should not help clients if they don't understand that they're doing. Looks like to much hustle for a wrong type annotation, which we can not just drop. Added dummy conversion to numpy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing it!
the system should not help clients if they don't understand that they're doing
I don't fully understand this part; the type hints come from the library and help the client build robust code through static type checking (eg mypy). If the types are imprecise or loose, the client is forced to implement extra logic or suppress the warnings.
What does this PR do?
Adds support of representing non simply connected sets with contours: main contour describes the outer border of the set and "holes" are represented as child contours lying inside the space bounded by the outer one.
Before submitting