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

Proposal: Make point first class citizens of Joy #2

Closed
anandology opened this issue Jun 7, 2021 · 9 comments
Closed

Proposal: Make point first class citizens of Joy #2

anandology opened this issue Jun 7, 2021 · 9 comments
Assignees

Comments

@anandology
Copy link
Contributor

Right now we specify just the x and y coordinates to the functions like x1, y1 or cx, cy etc. How about passing points instead of individual coordinates?

The new API would become:

circle(center=(100, 100), radius=50)
line(start=(10, 10), end=(50, 50))
rect(topleft=(50, 50), width=50, height=100)
rect(center=(0, 0), width=50, height=100)

As mentioned in #1, these shapes will have points available as attributes.

>>> c = circle(center=(100, 100), radius=50)
>>> print(c.center, c.right)

And we could use the same for transformations.

z = line(start=(0, 0), end=(100, 0))
rotate(z, angle=45, anchor=z.end)
@anandology anandology self-assigned this Jun 7, 2021
@anandology
Copy link
Contributor Author

anandology commented Jun 7, 2021

What about scale? As of now, it takes two numbers sx and sy. Will that stay the same or become some kind of vector?

@amitkaps
Copy link

amitkaps commented Jun 7, 2021

I like making Point as first class citizen. Also, the internal api can then be based on Point & Matrix transforms

Point = Num Num
Angle = Degree

Shape = Polyline [Point]
     | Polygon [Point]
     | Ellipse Point, Width, Height
     | Transform Shape
     | Group [Shape]
     | Colored Shape Color

//  General Open Shape
Polyline:: [Point] -> Shape
// Named Case
Line:: Point Point -> Shape

//  General Closed Shape
Polygon:: [Point] -> Shape
// Named Case
Rect:: Point, Point, Point, Point -> Shape

// General Open Arc (maybe later)
Arc:: [Point] Radius StartAngle EndAngle Direction -> Shape
// Named Case  
Ellipse:: Point Width Height -> Shape
Circle:: Point Radius -> Shape

// General Transform Matrix
Transform:: [ScaleX, SkewY, SkewX, ScaleY, TranslateX, TranslateY] Shape -> Shape
// Specific Transforms
Scale:: ..
Rotate:: ...
Skew:: ...
Translate:: ...

@anandology
Copy link
Contributor Author

Rect:: Point, Point, Point, Point -> Shape

This will be Polygon of 4 sides, not a rectangle. I think we need Rect:: Point Width Height. I think specifying rectangles also using the center point is a simpler approach than specifying the top-left corner.

Also, my worry is that this may create difficulty and more syntax for newbies.

circle(100, 100, 50)

vs.

circle(center=(100, 100), radius=50)

The other option is to relax the named arguments.

p = (100, 100)
circle(p, 50)

Thoughts?

@amitkaps
Copy link

amitkaps commented Jun 7, 2021

What I sketched earlier was the internal api for it and not the external api. You can probably implement it many different ways so that is not the main point.

Making Point as a first-class citizen for me means - I should not only be able to use Point to construct Shape but also be able to get them back from a Shape. The challenge is to define what are meaningful points for a shape.

point1 = Point(x=0, y=0)
point2 = Point(x=100, y=50)

For Line is a linear undirected segment between two points and even after transform should be easy to get back line.points and line.center. The generic case would be a Polyline with Polyline.points as the meaningful option.

line1 = Line(start=point1, end=point2)
line1.points
# (0,0), (100,50)
line1.center
# (50,25)

For Rect you can instantiate it with center, width & height. Now the meaningful point are the 4 vertex point and maybe the center. But, once you apply Transform especially Skew it will no longer be a rectangle but a 4-sided Polygon. So the generic shape is a n-ordered points joined (and closed) by n-linear segment. Hence, internally the Rect is a special case of Polygon. That is all I was saying.

rect1 = Rect(center=Point(x=50, y =25), width=100, height=50)
rect1.points 
# (0, 0), (100, 0), (100, 50), (0, 50)
rect1.center
# (50, 25)

transform1 = SkewX(30)
shape1 = rect1 | transform1
show(shape1)
# <rect x="0" y="0" width="100" height="50"  transform="skewX(30)" /> 
# which is a parallelogram 
shape1.points, shape1.center
# ???

This is useful because then I can manipulate individual points of a shape to experiment with more expressive ideas.

@anandology
Copy link
Contributor Author

For Line is a linear undirected segment between two points and even after transform should be easy to get back line.points and line.center

Yes, that's what I was thinking as well.

I agree adding point would be useful, but it is also adding complexity. The hello world is now:

p = Point(20, 30)
c = Circle(center=p, radius=5)
show(c)

And the following is not easy to read:

rect1 = Rect(center=Point(x=50, y =25), width=100, height=50)

I would rather prefer:

rect1 = Rect(center=(50, 25), width=100, height=50)

and convert (50, 25) to a Point internally. I understand the purity part of it, but it feels a bit too long.

Anyway, let me give it a shot and see how it turn up.

@anandology
Copy link
Contributor Author

I'm having second thoughts about the | syntax. While I like that and it is elegant, I'm not sure if that is necessarily a good for new programmers. It is a new syntax and they will not be able to write their own code that can work with | operator without using the functionality in Joy. In a sense it feels like the library is disempowering the learners.

Thoughts?

@anandology
Copy link
Contributor Author

I write down both the APIs to compare one against the other.

https://gist.github.com/anandology/a02041798553736c7a326f49eddc22ec

I think the new version wins by a huge margin. It is a lot more coherent. Also, I've replaced combine function with + operator. That makes it very clean. There will be two operators in Joy: + for combining shapes and | for applying transformations.

@anandology
Copy link
Contributor Author

anandology commented Jun 8, 2021

In the new scheme, what will be the arguments to the Text node?

t = Text(text="Hello")
t = Text(value="Hello")
t = Text(contents="Hello")

The first one is confusing because the word text is appearing multiple times. I'm not sure either value or contents is the right word.

@anandology
Copy link
Contributor Author

In the new scheme, what will be the arguments to the Text node?

t = Text(text="Hello")
t = Text(value="Hello")
t = Text(contents="Hello")

The first one is confusing because the word text is appearing multiple times. I'm not sure either value or contents is the right word.

Moved this disucussion to #6.

anandology added a commit that referenced this issue Jun 8, 2021
Rewrote the entire user facing API of the library.

Major changes:
- all the shapes and transformations are in TitleCase: Circle, Rectangle etc.
- The + operator is used to combine shapes
- The | operator is used to apply transformations, allowing chaining of transformations
- Using Point object wherever makes sense instead of just x and y

Issue #2
anandology added a commit that referenced this issue Jun 9, 2021
Rewrote the entire user facing API of the library.

Major changes:
- all the shapes and transformations are in TitleCase: Circle, Rectangle etc.
- The + operator is used to combine shapes
- The | operator is used to apply transformations, allowing chaining of transformations
- Using Point object wherever makes sense instead of just x and y

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

No branches or pull requests

2 participants