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

Improvements for Measure types in the plot_aes #1358

Merged
merged 2 commits into from
Dec 19, 2019

Conversation

Mattriks
Copy link
Member

@Mattriks Mattriks commented Dec 1, 2019

  • I've added and/or updated the unit tests
  • I've run the regression tests
  • I've built the docs and confirmed these changes don't cause new errors
  • Move 2 functions (currently in this PR) to Measures.jl
  • I've squash'ed or fixup'ed junk commits with git-rebase

This PR:

Example

# Examples from #1357
p1 = plot(xmin=[.25], xmax=[.75], Geom.band, color=[colorant"red"])
p2 = plot(xmin=[.25], xmax=[.75], Geom.band, color=[colorant"red"],
 layer(Geom.rect, xmin=[0], xmax=[1], ymin=[0], ymax=[1], color=[colorant"blue"]))
p3 = plot(x=[2,4], y=[2,4], size=[1.414], color=[colorant"gold"],
    Coord.cartesian(fixed=true) )
hstack(p1, p2, p3)

issue1357

Note the y-axis on plot p1: the Measure units appear because those are the only units provided (cf. plot p2).

@codecov-io
Copy link

codecov-io commented Dec 1, 2019

Codecov Report

Merging #1358 into master will decrease coverage by 0.07%.
The diff coverage is 95.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1358      +/-   ##
==========================================
- Coverage    90.6%   90.53%   -0.08%     
==========================================
  Files          38       38              
  Lines        3961     3950      -11     
==========================================
- Hits         3589     3576      -13     
- Misses        372      374       +2
Impacted Files Coverage Δ
src/coord.jl 91.57% <100%> (+0.18%) ⬆️
src/statistics.jl 96.93% <100%> (+0.04%) ⬆️
src/aesthetics.jl 81.51% <66.66%> (-0.39%) ⬇️
src/ticks.jl 90.32% <0%> (-1.49%) ⬇️

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 bb210ab...74f1310. Read the comment docs.

@tlnagy
Copy link
Member

tlnagy commented Dec 13, 2019

I tagged a new version of Measures, which should be out soon with your changes.

ref: JuliaRegistries/General#6638

@Mattriks
Copy link
Member Author

Mattriks commented Dec 15, 2019

Thanks! I ran the regression testing and found 2 diffs. I show the devel-output here.
i) The first one is because the Measure units now appear on the respective axes (as expected):

p1 = evalfile(dirname(pathof(Gadfly))*"/../test/testscripts/hvband.jl")

hvband

ii) The second one (below) is unexpected (but correct). Here Gadfly turns the categorical sizes into integers 1:3, so the {x,y} aesthetic ranges get expanded by ±3. There are current bugs in the way the size aesthetic is handled. e.g. Scale.size_continuous(minvalue= , maxvalue= ) should rescale the size aesthetic to 0-1. Similarly there needs to be a better design outline for how discrete sizes should be treated. I leave that to another PR.

p2 = evalfile(dirname(pathof(Gadfly))*"/../test/testscripts/point_size_categorical.jl")

point_size_categorical

@Mattriks
Copy link
Member Author

This is ready to merge, if there are no other comments.

@bjarthur bjarthur merged commit 8615883 into GiovineItalia:master Dec 19, 2019
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.

The function apply_statistic_typed doesn't handle type Measure
4 participants