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

Guide.shapekey #1156

Merged
merged 1 commit into from
Jul 1, 2018
Merged

Guide.shapekey #1156

merged 1 commit into from
Jul 1, 2018

Conversation

Mattriks
Copy link
Member

@Mattriks Mattriks commented May 28, 2018

  • I've updated the documentation to reflect these changes
  • I've added an entry to NEWS.md
  • I've added and/or updated the unit tests
  • I've run the regression tests
  • I've squash'ed or fixup'ed junk commits with git-rebase
  • I've built the docs and confirmed these changes don't cause new errors

Guide-ageddon

This is the first step in Guide-ageddon (and closes #1133). This PR implements:

  • Guide.shapekey(title=, labels=, pos=)
  • which generates a colored ShapeKey if shape and color are mapped to the same variable name.
  • Theme(key_swatch_color=), useful if shape and color are mapped to different variables.
srand(123)
theme1 = Theme(point_size=3mm)
coord1 = Coord.cartesian(xmin=0.0, xmax=6.0)
D = DataFrame(x=1:5, y=rand(5), V1=["A","A","B","B","D"], V2 = string.([1,2,2,3,3])  )

pa = plot(x=1:5, y=[0.77, 0.94, 0.67, 0.39, 0.31], shape=["A","A","B","B","D"], theme1, coord1,
    Guide.shapekey(title="Key",labels=["α","β","δ"]),
    Guide.title("Guide.shapekey") )

# if shape==color, and you change levels, there are possible ways to auto-update the levels in Scale.color_discrete
# but for now, you have to do it manually:
pb = plot(D, x=:x, y=:y, shape=:V1, color=:V1,
        Scale.shape_discrete(levels=["D","A","B"]),     
        Scale.color_discrete(levels=["D","A","B"]), 
        theme1, coord1, Guide.title("Shape==Color") )

pc = plot(D, x=:x, y=:y, shape=:V1, color=:V2,  coord1,
        Guide.colorkey(title="Color", pos=[0.75w,-0.25h]),
        Guide.shapekey(title="Shape "),
        Theme(point_size=3mm, key_swatch_color="slategrey"),
        Guide.title("Shape!=Color")
        )
hstack(pa,pb,pc)

shapekey

To do in this PR:

  • Investigate auto-updating Scale.color_discrete, if shape==color, and you change Guide.shapekey(levels=) (as in plot b above)

Future PRs (in no particular order):

  • Currently, if you have two keys (e.g. shape and color) positioned outside the plot (left or right) they will stack horizontally not vertically. I leave vertical stacking of left or right keys (and horizontal stacking of top or bottom-positioned keys) to a separate PR.
  • Update Guide.colorkey
  • Guide.sizekey, Guide.linekey
  • Update and replace Guide.manual_color_key with Guide.manual_key

@codecov-io
Copy link

codecov-io commented May 28, 2018

Codecov Report

Merging #1156 into master will increase coverage by 0.31%.
The diff coverage is 86.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1156      +/-   ##
==========================================
+ Coverage   87.06%   87.37%   +0.31%     
==========================================
  Files          33       36       +3     
  Lines        3981     4063      +82     
==========================================
+ Hits         3466     3550      +84     
+ Misses        515      513       -2
Impacted Files Coverage Δ
src/aesthetics.jl 82.35% <ø> (ø) ⬆️
src/guide.jl 100% <ø> (+14.07%) ⬆️
src/theme.jl 67.3% <ø> (ø) ⬆️
src/guide/misc.jl 100% <100%> (ø)
src/Gadfly.jl 79.42% <100%> (+1.7%) ⬆️
src/guide/plot.jl 85.02% <85.02%> (ø)
src/guide/keys.jl 86.52% <86.52%> (ø)
src/statistics.jl 93.52% <0%> (-0.02%) ⬇️
... and 25 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 f922f4d...071831f. Read the comment docs.

@bjarthur
Copy link
Member

this will be great to have. any thoughts on legends for size_{continuous,discrete}?

@Mattriks
Copy link
Member Author

See list of Future PRs above - Guide.sizekey is there. I think a sizekey should be easy, because even when the size variable is continuous, the key still contains discrete-size circles.

Note that I'm planning to make all discrete keys use render_discrete_key (new in this PR), which will contain keywords args for color, shape, size. If the keyword arg is not provided, then the relevant default from Theme is used.

@bjarthur
Copy link
Member

@Mattriks #1116 is ready to merge. but there will be lots of conflicts with this PR regarding your splitting of guides.jl. i think it would be best to do that in a separate PR.

also, if you think you'll be ready to merge this PR soon, i could wait to merge mine until afterwards. could work on seaborn-style thumbnails in the meantime. let me know.

@Mattriks Mattriks force-pushed the shapekey branch 3 times, most recently from 47aae36 to 20e030a Compare June 7, 2018 13:56
@Mattriks
Copy link
Member Author

Mattriks commented Jun 7, 2018

Rebased on master. The tests all pass locally for me.
Now that I've rebased, I can make the shapekey documentation consistent with the new "gallery format".

@bjarthur
Copy link
Member

lgtm except reformatting the docs to the new "gallery format". let's get this merged before 0.7!

@Mattriks
Copy link
Member Author

Mattriks commented Jul 1, 2018

I've updated the docs now, and ran the regression tests. There are diffs in any testfile that is prefixed "point_shape" or "point_*_shape" i.e. a shape key is added as expected. Tests passed for Julia 0.6 on Travis.

@bjarthur bjarthur merged commit 75fa855 into GiovineItalia:master Jul 1, 2018
@bjarthur
Copy link
Member

bjarthur commented Jul 1, 2018

thanks! looking forward to "Future PRs" you list in the OP.

@bjarthur
Copy link
Member

bjarthur commented Jul 7, 2018

if the length of the shape aesthetic is one would it make sense to not show the shape key? see http://gadflyjl.org/latest/gallery/shapes.html

@Mattriks
Copy link
Member Author

Mattriks commented Jul 8, 2018

Yes, I'll have to fix that, so it is consistent with the behaviour of the color aesthetic, e.g. color=[colorant"orange"]

@bjarthur bjarthur mentioned this pull request Jul 16, 2018
4 tasks
@bjarthur
Copy link
Member

@Mattriks what's the best way to fix the shape legend appearing when the length of shape is just one? i'm going to either have to fix it or change all my scripts, so figure i might as well tackle this.

@Mattriks
Copy link
Member Author

Either:

  • let me finish Fixes #1126 #1175 without the deprecation, or
  • show me a way of doing a Theme field deprecation which works,
    and I'll get back to the above shape issue. 🙂

I also started working on Guide.linekey, so I'm thinking about a holistic solution when the aesthetic length is one.

@bjarthur
Copy link
Member

bjarthur commented Aug 2, 2018

great to hear about linekey!

ideally it'd be best if we could fix this regression and tag a new point release before introducing any breaking changes like that in #1175.

@tlnagy
Copy link
Member

tlnagy commented Aug 2, 2018

I agree with @bjarthur here. I think lets fix this regression first and tag a new point release, merge #1175 and company and push one last feature release before we drop support for 0.6 on master.

@tlnagy tlnagy mentioned this pull request Aug 18, 2018
6 tasks
@bjarthur
Copy link
Member

@Mattriks any chance you'll have time in the near future to get the shape legend to not appear when the length of the shape aesthetic is just one? would be nice to get this fixed before we drop support for julia 0.6. thanks.

@Mattriks Mattriks mentioned this pull request Sep 17, 2018
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.

A legend should be generated automatically when a shape aesthetic is bound
4 participants