CGAL
Disambiguate offset_polygon_2() from create_offset_polygons_2()
The first function is in 2D Minkowski Sums, the second function in Straight Skeletons.
We suggest to replace the latter by straight_skeleton_offset_polygons_2()
We stick to plural as it does not generate a single polygon with holes. We drop the prefix create_ as that is what functions do.
As no one complained in a year, I think we can assume this change can be made.
@afabri Can you take care of this?
Look at the entire set of functions that this package provide.
/_____/) o /_________ __ // (____ ( ( ( (/ (/-(-'_(/ _/
On Mon, Sep 28, 2015 at 5:30 PM, Andreas Fabri [email protected] wrote:
The first function is in 2D Minkowski Sums, the second function in Straight Skeletons.
We suggest to replace the latter by straight_skeleton_offset_polygons_2()
We stick to plural as it does not generate a single polygon with holes. We drop the prefix create_ as that is what functions do.
— Reply to this email directly or view it on GitHub https://github.com/CGAL/cgal/issues/371.
Good point, it seems that both packages have their own name standards and we can't change one single function without changing the entire sets of function of one or the other packages (which seems an overkill to me).
Unless we can figure out a way of disambiguating these two functions without breaking the consistency of packages, I will close this issue.
As no one suggested a better solution, I am closing this issue. Please open a small feature if you want to change the API.
It is still an issue and it is unresolved. So I don’t see what is gained by closing it.
On 28 Oct 2016, at 07:58, Simon Giraudot <[email protected]mailto:[email protected]> wrote:
As no one suggested a better solution, I am closing this issue. Please open a small feature if you want to change the API.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/CGAL/cgal/issues/371#issuecomment-256842654, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AKyZmvKT1aplLViCXXqLV8pGYnCSVYiRks5q4Y8WgaJpZM4GE9ER.
Alright, I'm reopening the issue but I don't see what is gained either by leaving an issue opened and not doing anything to solve it: this issue has been opened for a year with barely anyone participating. So far, the only proposition was from @afabri:
We suggest to replace the latter by straight_skeleton_offset_polygons_2()
Which is, as @efifogel pointed, not a valid solution as it breaks the consistency within the Straight Skeleton package. In my opinion, the fact that this is an issue is debatable, both packages have their own name standards and it seems unlikely that a user will mistake one from the other, but I'm not in the best position to judge.
@efifogel and @fcacciola, you are the respective maintainers of Straight_skeleton_2 and of Minkowski_sum_2: do you agree there is problem and if so, do you see any way we could solve it?
There is a problem. BW, it's not only with offset_polygon_2(): A function called intersection_2() exists in the Kernel and in the 2D Boolean Op. package. There are 2 possible solutions I can think of: renaming and adding namespaces (or both). Introducing namespaces can be done globally to all package in a new major version.
/_____/) o /_________ __ // (____ ( ( ( (/ (/-(-'_(/ _/
On Fri, Oct 28, 2016 at 10:16 AM, Simon Giraudot [email protected] wrote:
Alright, I'm reopening the issue but I don't see what is gained either by leaving an issue opened and not doing anything to solve it: this issue has been opened for a year with barely anyone participating. So far, the only proposition was from @afabri https://github.com/afabri:
We suggest to replace the latter by straight_skeleton_offset_polygons_2()
Which is, as @efifogel https://github.com/efifogel pointed, not a valid solution as it breaks the consistency within the Straight Skeleton package http://doc.cgal.org/latest/Straight_skeleton_2/group__PkgStraightSkeleton2Functions.html#gaf85a09dc47cb9bb9e2ef28e00fc905ff. In my opinion, the fact that this is an issue is debatable, both packages have their own name standards and it seems unlikely that a user will mistake one from the other, but I'm not in the best position to judge.
@efifogel https://github.com/efifogel and @fcacciola https://github.com/fcacciola, you are the respective maintainers of Straight_skeleton_2 and of Minkowski_sum_2: do you agree there is problem and if so, do you see any way we could solve it?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CGAL/cgal/issues/371#issuecomment-256853066, or mute the thread https://github.com/notifications/unsubscribe-auth/AGoWuOnetIe7UOpNnG9RMq1VgapVKVPEks5q4aFmgaJpZM4GE9ER .
On 29 Oct 2016, at 09:17, Efi Fogel [email protected] wrote:
There is a problem. BW, it's not only with offset_polygon_2(): A function called intersection_2() exists in the Kernel and in the 2D Boolean Op. package.
This is less problematic because the intersection operation is conceptually the same in both cases. Otoh, the two offset_polygons are quite different...
When we discussed the issue first, we thought that renaming the straight skeleton incarnation makes sense because this is a straight skeleton offset (i.e. very specific to the straight skeleton) rather than a polygon (which, of course, it also happens to be as well, somehow).
On Sat, Oct 29, 2016 at 10:42 AM, Michael Hoffmann <[email protected]
wrote:
On 29 Oct 2016, at 09:17, Efi Fogel [email protected] wrote:
There is a problem. BW, it's not only with offset_polygon_2(): A function called intersection_2() exists in the Kernel and in the 2D Boolean Op. package.
This is less problematic because the intersection operation is conceptually the same in both cases.
Actually, they are not; the version in the "2D Regularized Boolean
Operations" is regularized (closure(interior(
Otoh, the two offset_polygons are quite different...
When we discussed the issue first, we thought that renaming the straight skeleton incarnation makes sense because this is a straight skeleton offset (i.e. very specific to the straight skeleton) rather than a polygon (which, of course, it also happens to be as well, somehow).
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CGAL/cgal/issues/371#issuecomment-257077231, or mute the thread https://github.com/notifications/unsubscribe-auth/AGoWuA7ys7UIZrrbKZ5tTC3gF1H-tpjHks5q4vj4gaJpZM4GE9ER .
There is a problem. BW, it's not only with offset_polygon_2(): A function called intersection_2() exists in the Kernel and in the 2D Boolean Op. package.
This is less problematic because the intersection operation is conceptually the same in both cases.
Actually, they are not; the version in the "2D Regularized Boolean Operations" is regularized (closure(interior(
)).
then maybe it should be renamed to regularized_intersection ?...
On Sat, Oct 29, 2016 at 11:35 AM, Michael Hoffmann <[email protected]
wrote:
There is a problem. BW, it's not only with offset_polygon_2(): A function called intersection_2() exists in the Kernel and in the 2D Boolean Op. package.
This is less problematic because the intersection operation is conceptually the same in both cases.
Actually, they are not; the version in the "2D Regularized Boolean Operations" is regularized (closure(interior(
)). then maybe it should be renamed to regularized_intersection ?...
Yes, perhaps. All functions there, and there are many, perform regularized operations. Non have the "regularized_" prefix. I think it would be better introducing a namespace. (Then, naturally, we need to keep versions without athe namespace for some time for backward compatibility.)
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CGAL/cgal/issues/371#issuecomment-257079311, or mute the thread https://github.com/notifications/unsubscribe-auth/AGoWuI9BhVaug9likbOVfEQwesDFQlFWks5q4wVNgaJpZM4GE9ER .
I almost missed this,
On Sat, Oct 29, 2016 at 6:33 AM, Efi Fogel [email protected] wrote:
On Sat, Oct 29, 2016 at 11:35 AM, Michael Hoffmann < [email protected]
wrote:
There is a problem. BW, it's not only with offset_polygon_2(): A function called intersection_2() exists in the Kernel and in the 2D Boolean Op. package.
This is less problematic because the intersection operation is conceptually the same in both cases.
Actually, they are not; the version in the "2D Regularized Boolean Operations" is regularized (closure(interior(
)). then maybe it should be renamed to regularized_intersection ?...
Yes, perhaps. All functions there, and there are many, perform regularized operations. Non have the "regularized_" prefix. I think it would be better introducing a namespace. (Then, naturally, we need to keep versions without athe namespace for some time for backward compatibility.)
I agree this really is a problem, and I think introducing a namespace would be the a reasonable solution.
But I'm not sure how to effectively go about this, in terms of backward compatibity.
Best
Fernando Cacciola SciSoft Consulting, Founder http://www.scisoft-consulting.com
If a namespace is introduced and we keep the names of functions and classes, using-declarations can be used to keep the backward-compatibility.
@efifogel Shall we introduce the new namespace for BO_2 functions while breaking compatibility in CGAL 5.0?
We should handle that together with #5284 that is dealing with the do_intersect() that is different from the one in Kernel as it is a regularized version.
Yes, Iet's do it. Last time we discussed it (on the phone) you suggested something that I cannot remember. Do you?
/_____/) o /_________ __ // (____ ( ( ( (/ (/-(-'_(/ _/
On Thu, 7 Oct 2021 at 16:18, Sebastien Loriot @.***> wrote:
We should handle that together with #5284 https://github.com/CGAL/cgal/pull/5284 that is dealing with the do_intersect() that is different from the one in Kernel as it is a regularized version.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CGAL/cgal/issues/371#issuecomment-937784923, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVBNOE22U252TBL5YK65XDUFWM4DANCNFSM4BQT2EIQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.