[GSoC'21] Convenience methods for objects #356

Sov-trotter · 2021-07-03T11:41:31Z

PR Checklist

If you are contributing to Javis.jl, please make sure you are able to check off each item on this list:

Did I update CHANGELOG.md with whatever changes/features I added with this PR?
Did I make sure to only change the part of the file where I introduced a new change/feature?
Did I cover all corner cases to be close to 100% test coverage (if applicable)?
Did I properly add Javis dependencies to the Project.toml + set an upper bound of the dependency (if applicable)?
Did I properly add test dependencies to the test directory (if applicable)?
Did I check relevant tutorials that may be affected by changes in this PR?
Did I clearly articulate why this PR was made the way it was and how it was made?

Link to relevant issue(s)
Closes #313 #130 #141

How did you address these issues with this PR? What methods did you use?
For now I have implemented abstractions for basics objects like,

So these objects basically wrap the (args...)->func declarations. It would be useful to have a method for a vector of such shapes.
This also has some potential in the direction of having internal flow of metadata.

For avoiding conflicts with luxor's and javis' already existing basic shapes, the methods have been named J<SHAPE> eg: JCircle,JStar

With convenience methods, support minimizing expressivity in frames:

codecov · 2021-07-03T11:56:32Z

Codecov Report

Merging #356 (2b273a5) into master (b7cfcb6) will increase coverage by 0.21%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #356      +/-   ##
==========================================
+ Coverage   95.99%   96.20%   +0.21%     
==========================================
  Files          27       35       +8     
  Lines        1372     1449      +77     
==========================================
+ Hits         1317     1394      +77     
  Misses         55       55

Impacted Files	Coverage Δ
src/Javis.jl	`96.39% <ø> (ø)`
src/structs/Object.jl	`100.00% <ø> (ø)`
src/shorthands/JBox.jl	`100.00% <100.00%> (ø)`
src/shorthands/JCircle.jl	`100.00% <100.00%> (ø)`
src/shorthands/JEllipse.jl	`100.00% <100.00%> (ø)`
src/shorthands/JLine.jl	`100.00% <100.00%> (ø)`
src/shorthands/JPoly.jl	`100.00% <100.00%> (ø)`
src/shorthands/JRect.jl	`100.00% <100.00%> (ø)`
src/shorthands/JShape.jl	`100.00% <100.00%> (ø)`
src/shorthands/JStar.jl	`100.00% <100.00%> (ø)`
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7cfcb6...2b273a5. Read the comment docs.

Sov-trotter · 2021-07-03T19:53:26Z

Test Script: https://gist.github.com/Sov-trotter/2df6181fd2ca2d2f121ecd7be490c2aa

Sov-trotter · 2021-07-03T20:41:59Z

@TheCedarPrince @Wikunia I can use a review at this point :)

Sov-trotter · 2021-07-03T21:55:10Z

@TheCedarPrince I remember us talking about what if someone wants to define some custom anonymous functions that are much detailed in functionality. Eg: the path! function here https://github.com/Wikunia/Javis.jl/blob/master/docs/src/tutorials/tutorial_1.md#mapping-an-orbit-

For that I have created a macro viz. JShape which allows one to wrap such instructions in a block.

somepath = Object(1:70,  Javis.@JShape begin
                                          sethue("blue")
                                          circle.(<out points>, 20, :fill)
                                          box(....)
                                          star(.....)
                                      end  )

JShape

Wikunia

Thanks for creating this PR @Sov-trotter I think it will simplify a lot of things in the future. Especially the JShape is a great addition. Is it possible to maybe make that accept keyword arguments as well? For example

@JShape begin
   ....
end color=:black radius=20

src/shorthands/JBox.jl

src/Javis.jl

Sov-trotter · 2021-07-04T10:04:32Z

@JLayer begin
   ....
end color=:black radius=20

Is it possible to maybe make that accept keyword arguments as well?

I think you meant JShape here? 😅

Yeah. That is possible. But I am not sure how we can use those arguments inside since we just evaluate the expression inside the anonymous function. https://github.com/Wikunia/Javis.jl/pull/356/files#diff-357a561d1b0f8639d39256acccb59064e0cdde58dc13775c9d8f9e3bcefbbf01R18

A better workaround would be if the one can do something like moving those arguments in the block so that the variables can be used inside long declarations.

@JShape begin
  color=:black 
  radius=20
.........
end

Wikunia · 2021-07-04T10:35:29Z

Oh yeah I meant JShape. Yeah I'm not that knowledgeable in macros 😕
Wondering whether one can parse the arguments and than evaluate those before calling eval
Something like:

macro JShape(body::Expr, expr)
    kwargs = (; expr.args[1] = expr.args[2])
    expr = quote
        (args...) -> begin 
            esc(
                for (key, value) in enumerate($kwargs)
                   @eval (($key) = ($value))
                end
            )
            eval($body)
        end
    end
    esc(expr)
end

which obviously doesn't work 😄

In your case one would need to parse the vars and values out of the begin end block and replace them with the kwargs, right?

Sov-trotter · 2021-07-04T10:40:06Z

Wondering whether one can parse the arguments and than evaluate those before calling eval

I think this should work.

@JShape begin
   sethue(color)
   circle(radius)
   line(O)
end color=:black radius=20

Is that what you have in mind?

Sov-trotter · 2021-07-04T10:42:35Z

In your case one would need to parse the vars and values out of the begin end block and replace them with the kwargs, right?

No. The arguments go inside the begin end block and are parsed along with the expression. So they are not essentially arguments but expressions.

Wikunia · 2021-07-04T11:06:22Z

@JShape begin
   sethue(color)
   circle(radius)
   line(O)
end color=:black radius=20

Is that what you have in mind?

yes that's what I thought.

Wikunia · 2021-07-04T11:07:27Z

In your case one would need to parse the vars and values out of the begin end block and replace them with the kwargs, right?

No. The arguments go inside the begin end block and are parsed along with the expression. So they are not essentially arguments but expressions.

But one would need to extract them to write the kwargs and would need to change the right hand side to actually use the kwarg value instead of the default.

Sov-trotter · 2021-07-04T12:49:15Z

@JShape begin
     sethue(color)
     circle(radius)
     line(O)
  end color=:black radius=20

Is that what you have in mind?

yes that's what I thought.

So one thing with this is that macros can't have keyword arguments, so it's not super useful to have fixed arguments(the only possible I can think of is color and action). I mean if one has longer code decrlaring variables inside the begin .. end block is much useful

Wikunia · 2021-07-04T19:21:25Z

I like your way of having it inside but assumed it's harder. One can parse them as keyword arguments though, so that shouldn't be the biggest problem

Wikunia

Experimenting a bit with this and ODEs a bit. Have just one small request for JLine for now. I can add that myself though as well.

@TheCedarPrince that is the start of what I was telling you about
The code: https://gist.github.com/Wikunia/4ca58b4c08da90978c0dcdee5b0c908a

Wikunia · 2021-08-05T15:43:33Z

src/shorthands/JLine.jl

+    (args...; color = color, p1 = p1, p2 = p2) -> _JLine(p1, p2, color)
+
+"""
+    JLine(pt::Point)


the color is missing in this docstring. I would also like to have linewidth for setline default should be 1.

TheCedarPrince

Looks really great! I edited the Tutorial some to make it a bit less technical but kept the same outline and general content. Also, the only major comment I left is I would just like to see a few tests for J-Objects being used with a Layer. Otherwise, when done, I think this is ready for merging!

src/shorthands/JBox.jl

TheCedarPrince · 2021-08-06T00:24:41Z

src/shorthands/JShape.jl

+Creates a custom shape based on the luxor instructions in the begin...end block
+```julia
+somepath1 = Object(@JShape begin
+                                    sethue(color)
+                                    poly(points, action, close= true)
+                                end action = :stroke color ="red" radius = 8 
+)
+```


Could you run JuliaFormatter on the code in this example? Somehow seems a bit off to me.

test/shorthands.jl

Wikunia · 2021-08-06T18:13:00Z

Would be lovely if you could check my docstring changes and overall structural change of some methods @TheCedarPrince

Sov-trotter · 2021-08-06T18:14:59Z

973e95c#diff-d35b42f4d138de274461c0f7ed00bfc89a3e90294bc64e30feae10a1f3ef2e84R13-R46 this is a really interesting way of using convenience functions.

Wikunia · 2021-08-06T18:16:14Z

973e95c#diff-d35b42f4d138de274461c0f7ed00bfc89a3e90294bc64e30feae10a1f3ef2e84R13-R46 this is a really interesting way of using convenience functions.

Haha yeah I really like that way to be honest. For this one it's a bit basic but can get quite handy sometimes.

TheCedarPrince

Looks good to me! One thing I am curious about is if we can do something down the road similar to matplotlib to handle the kwargs found in different functions - especially the J-Objects here (see **kwargs here: https://matplotlib.org/devdocs/api/_as_gen/matplotlib.pyplot.plot.html). I wonder if Documenter is smart enough to render documentation for a function's kwargs. I'll investigate.

Also, I was going to say that I wasn't exactly a fan of the 1., 2., etc. docstrings at first, but I think they make a lot of sense now. Especially looking at JEllipse - it makes a lot of sense. I think this is set to merge - great work @Sov-trotter and @Wikunia !

P.S. Made a small commit to fix the docstring of the @JShape macro.

Wikunia · 2021-08-07T07:13:46Z

Thanks for the review. Can you clarify a bit what you mean by the keyword argument thing?

Sov-trotter added 3 commits July 3, 2021 00:50

Implement JLine

6036d4a

Implement JCircle

fe02e0b

Simpler non--metadata implementation

2e8b778

Sov-trotter added 4 commits July 3, 2021 21:52

JBox and JRect

1b1d914

JEllipse

6e3ed1b

JStar

8389926

JPoly

3bf76f1

Sov-trotter added 3 commits July 4, 2021 01:23

Allow for polygons

351e5d8

Cleanup

6240011

Add docstrings

c89014b

Fix include

f7efb16

Sov-trotter changed the title ~~Convenience methods for objects~~ [GSoC'21] Convenience methods for objects Jul 3, 2021

JShape macro

a88f686

JShape

Wikunia reviewed Jul 4, 2021

View reviewed changes

src/shorthands/JBox.jl Outdated Show resolved Hide resolved

src/shorthands/JBox.jl Outdated Show resolved Hide resolved

src/Javis.jl Show resolved Hide resolved

Sov-trotter added 3 commits July 4, 2021 20:04

fix change() action

5ab9bf2

Return corner point for box and rect by default

f8f7c40

nit

7f9b5b8

Allow for kwargs in jshape

a78f738

Wikunia added 4 commits August 3, 2021 20:58

mention the functions in the changelog

8ceed59

set frames before Background to reuse frames there

9946ee7

fix tutorial links after the shifting

9f58ed9

Merge branch 'master' into convenience_objectsv2

7f9605f

Wikunia reviewed Aug 5, 2021

View reviewed changes

Clarified language and cleaned up outline of the tutorial

4d21430

TheCedarPrince reviewed Aug 6, 2021

View reviewed changes

Wikunia added 14 commits August 6, 2021 17:05

Merge branch 'master' into convenience_objectsv2

298afb0

test @jshape in layers

973e95c

typo fix in Changelog

c2cb0d9

format

86334e5

JShape format docstring and added explanation

41de4d3

JCircle restructure and support for linewidth

2d350a0

JLine supports linewidth

cac9b43

JLine docstring change + remove ReTest accident

1f3b9cb

format

e46d8a7

linewidth for JPoly

e41c044

JRect removed vertices, added linewidth, fixed color

6b420ba

format

8179d7d

JStar without vertices + added linewidth

cb5e34e

allow linewidth for ellipse + test linewidth using change

b34aaca

Wikunia requested a review from TheCedarPrince August 6, 2021 18:13

Wikunia and others added 2 commits August 6, 2021 23:24

fixed tests and refs

ae0ed56

Formatted docstring some

2b273a5

TheCedarPrince approved these changes Aug 7, 2021

View reviewed changes

Wikunia merged commit 04fdba6 into JuliaAnimators:master Aug 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GSoC'21] Convenience methods for objects #356

[GSoC'21] Convenience methods for objects #356

Sov-trotter commented Jul 3, 2021 •

edited

Loading

codecov bot commented Jul 3, 2021 •

edited

Loading

Sov-trotter commented Jul 3, 2021

Sov-trotter commented Jul 3, 2021

Sov-trotter commented Jul 3, 2021 •

edited

Loading

Wikunia left a comment •

edited

Loading

Sov-trotter commented Jul 4, 2021 •

edited

Loading

Wikunia commented Jul 4, 2021

Sov-trotter commented Jul 4, 2021 •

edited

Loading

Sov-trotter commented Jul 4, 2021

Wikunia commented Jul 4, 2021

Wikunia commented Jul 4, 2021

Sov-trotter commented Jul 4, 2021 •

edited

Loading

Wikunia commented Jul 4, 2021

Wikunia left a comment •

edited

Loading

Wikunia Aug 5, 2021

TheCedarPrince left a comment

TheCedarPrince Aug 6, 2021

Wikunia commented Aug 6, 2021

Sov-trotter commented Aug 6, 2021

Wikunia commented Aug 6, 2021

TheCedarPrince left a comment

Wikunia commented Aug 7, 2021

[GSoC'21] Convenience methods for objects #356

[GSoC'21] Convenience methods for objects #356

Conversation

Sov-trotter commented Jul 3, 2021 • edited Loading

PR Checklist

codecov bot commented Jul 3, 2021 • edited Loading

Codecov Report

Sov-trotter commented Jul 3, 2021

Sov-trotter commented Jul 3, 2021

Sov-trotter commented Jul 3, 2021 • edited Loading

Wikunia left a comment • edited Loading

Choose a reason for hiding this comment

Sov-trotter commented Jul 4, 2021 • edited Loading

Wikunia commented Jul 4, 2021

Sov-trotter commented Jul 4, 2021 • edited Loading

Sov-trotter commented Jul 4, 2021

Wikunia commented Jul 4, 2021

Wikunia commented Jul 4, 2021

Sov-trotter commented Jul 4, 2021 • edited Loading

Wikunia commented Jul 4, 2021

Wikunia left a comment • edited Loading

Choose a reason for hiding this comment

Wikunia Aug 5, 2021

Choose a reason for hiding this comment

TheCedarPrince left a comment

Choose a reason for hiding this comment

TheCedarPrince Aug 6, 2021

Choose a reason for hiding this comment

Wikunia commented Aug 6, 2021

Sov-trotter commented Aug 6, 2021

Wikunia commented Aug 6, 2021

TheCedarPrince left a comment

Choose a reason for hiding this comment

Wikunia commented Aug 7, 2021

Sov-trotter commented Jul 3, 2021 •

edited

Loading

codecov bot commented Jul 3, 2021 •

edited

Loading

Sov-trotter commented Jul 3, 2021 •

edited

Loading

Wikunia left a comment •

edited

Loading

Sov-trotter commented Jul 4, 2021 •

edited

Loading

Sov-trotter commented Jul 4, 2021 •

edited

Loading

Sov-trotter commented Jul 4, 2021 •

edited

Loading

Wikunia left a comment •

edited

Loading