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

Segment Geometry #823

Merged
merged 1 commit into from
Sep 1, 2016
Merged

Segment Geometry #823

merged 1 commit into from
Sep 1, 2016

Conversation

Mattriks
Copy link
Member

A geometry for line segments/vectors/arrows. There is an example in the file geom_segment.md. This feature was previously requested in Issues #608 and #791. My current code will work for regular plots, but not yet for subplot_grid. I'll attempt to implement that sometime soon!

@tlnagy
Copy link
Member

tlnagy commented Aug 22, 2016

Hey @Mattriks, if you have time, can you rebase this on the latest master? Lets check if it works with all the recent changes and make sure it's all okay on 0.5. I think this would be a cool geometry to have.

@coveralls
Copy link

coveralls commented Aug 23, 2016

Coverage Status

Coverage decreased (-0.4%) to 65.072% when pulling 05d8ef1 on Mattriks:Geom_segment into 4a36837 on dcjones:master.

@coveralls
Copy link

coveralls commented Aug 23, 2016

Coverage Status

Coverage decreased (-0.4%) to 65.072% when pulling 05d8ef1 on Mattriks:Geom_segment into 4a36837 on dcjones:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 65.072% when pulling ef675b3 on Mattriks:Geom_segment into 4a36837 on dcjones:master.

2 similar comments
@coveralls
Copy link

coveralls commented Aug 23, 2016

Coverage Status

Coverage decreased (-0.4%) to 65.072% when pulling ef675b3 on Mattriks:Geom_segment into 4a36837 on dcjones:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 65.072% when pulling ef675b3 on Mattriks:Geom_segment into 4a36837 on dcjones:master.

@Mattriks
Copy link
Member Author

I've tested this now on Julia 0.5-rc2. The example in the file geom_segment.md works. Also, this Segment Geometry works when using with Geom.subplot_grid.

@lobingera
Copy link

lobingera commented Aug 23, 2016

i wanted to report an issue on this issue, but it was just a githup hiccup...

@tlnagy
Copy link
Member

tlnagy commented Aug 23, 2016

@Mattriks Awesome. Could you add some tests?

@tlnagy
Copy link
Member

tlnagy commented Aug 24, 2016

I got to play around with this and looks good!

The only thing is that the arrowheads when calling Geom.vector look a little funky:

plot(x=rand(1:10, 10), y=rand(1:10, 10), xend=rand(1:10, 10), yend=rand(1:10, 10), Geom.vector)

screen shot 2016-08-24 at 12 26 59

And they don't behave properly when one of the axes is logged

screen shot 2016-08-24 at 12 30 13

@tlnagy
Copy link
Member

tlnagy commented Aug 24, 2016

Actually, the first one is caused by not having a square axis ratio. But in general, I would think that you don't want scaling and transformations to apply to the arrowheads.

@Mattriks
Copy link
Member Author

Mattriks commented Aug 28, 2016

I've implemented a new and improved Geom.vector (I've also included tests). In Gadfly, the current situation is that no information about Scales is passed to the geometry render functions. But for rendering vectors properly, info about the x and y scales must be provided. That is, the user must manually provide a Scale object (with a minvalue and maxvalue) for both axes. I will look into automating this in the future. Here is a current working example:

srand(1234)
D = convert(DataFrame, 99*rand(4, 4)+0.5)
xsc  = Scale.x_continuous(minvalue=0.0, maxvalue=100)
xlogsc  = Scale.x_log10(minvalue=1, maxvalue=100)
ysc  = Scale.y_continuous(minvalue=0, maxvalue=100)
xsqrtsc  = Scale.x_sqrt(minvalue=0, maxvalue=100)
layer1 = layer(x=:x1, y=:x2, xend=:x3, yend=:x4, Geom.vector)

pa = plot(D, layer1, xsc, ysc,
Guide.title("Plain scales"), Guide.ylabel(nothing), Guide.xlabel(" "))
pb = plot(D, layer1, xlogsc, ysc,
Guide.title("Semi-log"), Guide.ylabel(nothing), Guide.xlabel("x"))
pc = plot(D, layer1, xsqrtsc, ysc,
Guide.title("√x"), Guide.ylabel(nothing), Guide.xlabel(" "))

gvec

@coveralls
Copy link

coveralls commented Aug 28, 2016

Coverage Status

Coverage increased (+0.3%) to 65.863% when pulling 1670e74 on Mattriks:Geom_segment into f6183ac on dcjones:master.

@tlnagy
Copy link
Member

tlnagy commented Aug 29, 2016

Very nice! What do you think about filled-in arrowheads? Whenever I think of vector fields, I always think of something like this:
2000px-vectorfield svg

(https://en.wikipedia.org/wiki/Vector_field)

A geometry for line segments/vectors/arrows. There is an example in the
file geom_segment.md.
@Mattriks
Copy link
Member Author

Some extra attributes (example below). Wrt gradient vectorfields, that could be implemented as function Geom.gradvectorfield(z, x, y) (returning a SegmentGeometry), just as Geom.contour(z,x,y) returns a LineGeometry. That's a feature for the future, need to get Geom.segment merged first.

srand(123)
D = convert(DataFrame, 99*rand(4, 4)+0.5)
D[:col] = ["a","b","c","d"]

xsc  = Scale.x_continuous(minvalue=0.0, maxvalue=100)
ysc  = Scale.y_continuous(minvalue=0.0, maxvalue=100)
theme1 = Theme(line_width=3pt, line_style=[4mm,1mm], key_position=:none)
lf(x) = layer(x=:x1, y=:x2, xend=:x3, yend=:x4, x, color=:col)

pa = plot(D, lf(Geom.vector), xsc, ysc, Theme(key_position=:none))
pb = plot(D, lf(Geom.vector(filled=true)), xsc, ysc, theme1)

vec2

@coveralls
Copy link

coveralls commented Aug 30, 2016

Coverage Status

Coverage increased (+0.3%) to 65.864% when pulling 9adbbf9 on Mattriks:Geom_segment into 30670b3 on dcjones:master.

@tlnagy
Copy link
Member

tlnagy commented Aug 30, 2016

That looks awesome. Personally, I like filled arrows to be the default, but that can changed in the future.

I've looked through this and I think it all looks good. LGTM. @shashi what do you think?


# Geom.vector requires information about scales

if geom.arrow
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the top-level if statement necessary because render doesn't get scale information right now? Because this looks a bit hacky.

@Mattriks
Copy link
Member Author

Mattriks commented Sep 1, 2016

Scales are needed to stop arrowheads looking strange. Segments without arrowheads are not affected i.e. Geom.segment(arrow=false)

@tlnagy
Copy link
Member

tlnagy commented Sep 1, 2016

I see. I guess eventually we'll want to get rid of that when we pass Scales to render.

@tlnagy
Copy link
Member

tlnagy commented Sep 1, 2016

Alright. LGTM.

@tlnagy tlnagy merged commit 81ef863 into GiovineItalia:master Sep 1, 2016
@bjarthur bjarthur mentioned this pull request Apr 7, 2017
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

Successfully merging this pull request may close these issues.

None yet

4 participants