Add Geom.candlestick

[iblislin]

Contributor checklist:

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

This PR

• add Geom.candlestick()

Example

using Gadfly, MarketData
ta = ohlc[1:40]
plot(
    x     = timestamp(ta),
    open  = values(ta.Open),
    high  = values(ta.High),
    low   = values(ta.Low),
    close = values(ta.Close),
    Geom.candlestick,
    Scale.color_discrete_manual("green", "red"))

TODO

• [x] ~inverse color, if close price < open price~ Just let user set the desired colour via Scale

[codecov-io]

Codecov Report

Merging #1215 into master will increase coverage by 0.44%. The diff coverage is 90%.

@@            Coverage Diff             @@
##           master    #1215      +/-   ##
==========================================
+ Coverage   90.31%   90.75%   +0.44%     
==========================================
  Files          39       39              
  Lines        4242     4512     +270     
==========================================
+ Hits         3831     4095     +264     
- Misses        411      417       +6

Impacted Files	Coverage Δ
src/aesthetics.jl	81.3% <ø> (ø)	:arrow_up:
src/geom/boxplot.jl	97.22% <90%> (-2.78%)	:arrow_down:
src/statistics.jl	97.19% <0%> (+0.27%)	:arrow_up:

---

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 3a2e115...4c2bbe1. Read the comment docs.

[iblislin]

I tried to plot a dataframe with 20 rows, got the following plot. The number of candlesticks is correct, but the spacing between them is weird. Any ideas?

https://gist.github.com/iblis17/7fc5a709eb201f8bbc6a7b6faa4479c4

[bjarthur]

the x-axis is a date, and it's skipping the weekends

[bjarthur]

needs a test, and maybe sth for the gallery (say TSLA after musk was charged by SEC?). otherwise, lgtm. @Mattriks @tlnagy thoughts?

[iblislin]

the x-axis is a date, and it's skipping the weekends

is it able to remove the weekends in the render function?

[iblislin]

needs a test, and maybe sth for the gallery (say TSLA after musk was charged by SEC?).

will do, but I need to figure out my TODO (inverse color) first.

[iblislin]

What kind of API design is suitable for this case: I want that user can configure a pair of colors for (1) open > close and (2) open < close. (because I found that the inverting color automatically is difficault in this case, that is, inverting the RGB of red != RGB of green. But in stock market, green and red are common for drawing candlestick.)

[Mattriks]

Here is the Gadfly way. The idea is to write an apply_statistic function for a CandlestickStatistic. You'll find examples in Gadfly's statistics.jl file, where the apply_statistic function adds a color_scale. The end of theapply_statistic function will look something like in the code below. To test, run the uncommented lines directly in julia, and look at the aes object. You'll see a color scale has been added, which (in this example) contains red and blue at the appropriate values of hlgroup. Magic!

# scales and aes are function arguments, which I simulate here
# You can use aes.open and aes.close, rather than the hinges which I use here 

scales = Dict(:color=>Scale.color_discrete_manual("red","blue"))
aes = Gadfly.Aesthetics()
aes.upper_hinge = rand(10)  
aes.lower_hinge = rand(10) 

# The apply_statistic function:
# function apply_statistic(stat::CandlestickStatistic, scales, coord, aes)

    hlgroup = aes.upper_hinge .> aes.lower_hinge
    color_scale = get(scales, :color, Scale.color_discrete())
    Scale.apply_scale(color_scale, [aes], Gadfly.Data(color = hlgroup))
# end    

aes.color

[Mattriks]

Also test this example (replace the appropriate lines above):

 scales = Dict{Symbol, Gadfly.ScaleElement}() 
 
 color_scale = get(scales, :color, Scale.color_discrete_manual("red","green"))

Does it make sense?

[iblislin]

cool, I ran them in my REPL. looks fine. And I just need time to understand what happened.

[iblislin]

This PR is ready for review.

I think the Travis CI failures are unrelated to this PR.

[bjarthur]

looks good to me. @Mattriks @tlnagy thoughts?

[Mattriks]

For PRs it's also good to show an example png of your new feature like this

[iblislin]

I added an example png.

[tlnagy]

@bjarthur and @Mattriks, this looks good to go to me.

[codecov-commenter]

Codecov Report

Merging #1215 into master will increase coverage by 0.02%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1215      +/-   ##
==========================================
+ Coverage   89.32%   89.35%   +0.02%     
==========================================
  Files          39       39              
  Lines        4395     4404       +9     
==========================================
+ Hits         3926     3935       +9     
  Misses        469      469              

Impacted Files	Coverage Δ
src/aesthetics.jl	77.77% <ø> (ø)
src/geom/boxplot.jl	100.00% <100.00%> (ø)
src/statistics.jl	96.47% <100.00%> (+0.01%)	:arrow_up:

---

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 d507b20...4366e97. Read the comment docs.

[iblislin]

Good to go?

[iblislin]

new example:

[bjarthur]

this looks good to me. @Mattriks ?

[Mattriks]

Hopefully I'll get around to looking at this soon (this week).

[iblislin]

any updates?

[codecov-io]

Codecov Report

Merging #1215 (c741f56) into master (904078a) will increase coverage by 0.02%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1215      +/-   ##
==========================================
+ Coverage   89.62%   89.64%   +0.02%     
==========================================
  Files          39       39              
  Lines        4596     4606      +10     
==========================================
+ Hits         4119     4129      +10     
  Misses        477      477              

Impacted Files	Coverage Δ
src/aesthetics.jl	78.94% <ø> (ø)
src/geom/boxplot.jl	100.00% <100.00%> (ø)
src/statistics.jl	96.48% <100.00%> (+0.01%)	:arrow_up:

---

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 904078a...c741f56. Read the comment docs.

[iblislin]

I resolved the conflict. Good to go?

[btbvoy]

@iblislin @Mattriks @bjarthur @tlnagy could you please share any reasons this PR left abandoned and whether there is any chance to make it happen? It looks like it was ready to be merged for quite some time, but due to a long period of inactivity there are some other conflicts. Thanks in advance

[bjarthur]

this still LGTM. @Mattriks fine by you?

to resolve the conflict, i think the dep on MarketData simply needs to be added to test/Project.toml, and the main Project.toml needs to be left unchanged.

[bjarthur]

accidentally merged while resolving conflict, but it looks good so i think it's okay. can always fix later if there's a problem.

i couldn't get the docs to build locally though, and the dev version online doesn't seem to have updated. unrelated to this PR though i think. will look into it later. the method signature of render_discrete_key seems to need to be expanded to AbstractString:

$ julia make.jl 
[ Info: SetupBuildDirectory: setting up build directory.
[ Info: Doctest: running doctests.
[ Info: ExpandTemplates: expanding markdown templates.
ERROR: LoadError: MethodError: no method matching render_discrete_key(::Vector{InlineStrings.String7}, ::Context, ::Measures.AbsoluteLength, ::Theme; sizes=Measures.AbsoluteLength[1.0583333333333331mm, 1.6462962962962961mm, 2.234259259259259mm, 2.822222222222222mm], colors=ColorTypes.Colorant[])
Closest candidates are:
  render_discrete_key(::Vector{String}, ::Context, ::Measure, ::Theme; colors, aes_color_label, shapes, sizes) at ~/.julia/dev/Gadfly/src/guide/keys.jl:72
Stacktrace:
  [1] render(guide::Gadfly.Guide.SizeKey, theme::Theme, aes::Gadfly.Aesthetics)
    @ Gadfly.Guide ~/.julia/dev/Gadfly/src/guide/keys.jl:247
  [2] render(guide::Gadfly.Guide.SizeKey, theme::Theme, aes::Gadfly.Aesthetics, dynamic::Bool)
    @ Gadfly.Guide ~/.julia/dev/Gadfly/src/guide.jl:143
  [3] render_prepared(plot::Plot, coord::Gadfly.Coord.Cartesian, plot_aes::Gadfly.Aesthetics, layer_aess::Vector{Gadfly.Aesthetics}, layer_stats::Vector{Vector{Gadfly.StatisticElement}}, layer_subplot_aess::Vector{Vector{Gadfly.Aesthetics}}, layer_subplot_datas::Vector{Vector{Gadfly.Data}}, scales::Dict{Symbol, Gadfly.ScaleElement}, guides::Vector{Gadfly.GuideElement}; table_only::Bool, dynamic::Bool)
    @ Gadfly ~/.julia/dev/Gadfly/src/Gadfly.jl:813
  [4] render_prepared(plot::Plot, coord::Gadfly.Coord.Cartesian, plot_aes::Gadfly.Aesthetics, layer_aess::Vector{Gadfly.Aesthetics}, layer_stats::Vector{Vector{Gadfly.StatisticElement}}, layer_subplot_aess::Vector{Vector{Gadfly.Aesthetics}}, layer_subplot_datas::Vector{Vector{Gadfly.Data}}, scales::Dict{Symbol, Gadfly.ScaleElement}, guides::Vector{Gadfly.GuideElement})
    @ Gadfly ~/.julia/dev/Gadfly/src/Gadfly.jl:782
  [5] render(plot::Plot)
    @ Gadfly ~/.julia/dev/Gadfly/src/Gadfly.jl:740
  [6] draw
    @ ~/.julia/dev/Gadfly/src/Gadfly.jl:843 [inlined]
  [7] show(io::IOBuffer, m::MIME{Symbol("text/html")}, p::Plot)
    @ Gadfly ~/.julia/dev/Gadfly/src/Gadfly.jl:933
  [8] __binrepr(m::MIME{Symbol("text/html")}, x::Plot, context::Nothing)
    @ Base.Multimedia ./multimedia.jl:159
  [9] _textrepr
    @ ./multimedia.jl:151 [inlined]
 [10] #stringmime#8
    @ /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Base64/src/Base64.jl:43 [inlined]
 [11] stringmime(m::MIME{Symbol("text/html")}, x::Plot)
    @ Base64 /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Base64/src/Base64.jl:43
 [12] display_dict(x::Plot)
    @ Documenter.Utilities ~/.julia/packages/Documenter/bFHi4/src/Utilities/Utilities.jl:627
 [13] #invokelatest#2
    @ ./essentials.jl:729 [inlined]
 [14] invokelatest
    @ ./essentials.jl:726 [inlined]
 [15] runner(#unused#::Type{Documenter.Expanders.ExampleBlocks}, x::Markdown.Code, page::Documenter.Documents.Page, doc::Documenter.Documents.Document)
    @ Documenter.Expanders ~/.julia/packages/Documenter/bFHi4/src/Expanders.jl:582
 [16] dispatch(::Type{Documenter.Expanders.ExpanderPipeline}, ::Markdown.Code, ::Vararg{Any})
    @ Documenter.Utilities.Selectors ~/.julia/packages/Documenter/bFHi4/src/Utilities/Selectors.jl:170
 [17] expand(doc::Documenter.Documents.Document)
    @ Documenter.Expanders ~/.julia/packages/Documenter/bFHi4/src/Expanders.jl:42
 [18] runner(#unused#::Type{Documenter.Builder.ExpandTemplates}, doc::Documenter.Documents.Document)
    @ Documenter.Builder ~/.julia/packages/Documenter/bFHi4/src/Builder.jl:227
 [19] dispatch(#unused#::Type{Documenter.Builder.DocumentPipeline}, x::Documenter.Documents.Document)
    @ Documenter.Utilities.Selectors ~/.julia/packages/Documenter/bFHi4/src/Utilities/Selectors.jl:170
 [20] #2
    @ ~/.julia/packages/Documenter/bFHi4/src/Documenter.jl:249 [inlined]
 [21] cd(f::Documenter.var"#2#3"{Documenter.Documents.Document}, dir::String)
    @ Base.Filesystem ./file.jl:112
 [22] makedocs(; debug::Bool, format::Documenter.Writers.HTMLWriter.HTML, kwargs::Base.Pairs{Symbol, Any, NTuple{4, Symbol}, NamedTuple{(:modules, :clean, :sitename, :pages), Tuple{Vector{Module}, Bool, String, Vector{Any}}}})
    @ Documenter ~/.julia/packages/Documenter/bFHi4/src/Documenter.jl:248
 [23] top-level scope
    @ ~/.julia/dev/Gadfly/docs/make.jl:3
in expression starting at /Users/arthurb/.julia/dev/Gadfly/docs/make.jl:3

[Mattriks]

@bjarthur I can add the change to render_discrete_key in my PR #1606.

[bjarthur]

thanks @iblislin . sorry this took so long

[bjarthur]

thanks @Mattriks . i think i got it though with https://github.com/GiovineItalia/Gadfly.jl/issues/1607. changes were needed for Compose too. let me know what you think and i'll merge.

[iblislin]

:tada: