Financial Chart: CandleStickRenderer, HighLowRenderer, Basic Financial API

[raven2cz]

Pull request according to issue #294. Screenshots attached in the issue. The samples and unit tests too.

[lgtm-com]

This pull request introduces 2 alerts when merging 2c9aba2 into 8caa280 - view on LGTM.com

new alerts:

• 2 for Potential input resource leak

[codecov]

Codecov Report

Merging #316 (b560516) into master (8caa280) will increase coverage by 0.67%.
The diff coverage is 87.39%.

@@             Coverage Diff              @@
##             master     #316      +/-   ##
============================================
+ Coverage     50.91%   51.58%   +0.67%     
- Complexity     7086     7266     +180     
============================================
  Files           382      393      +11     
  Lines         40501    41072     +571     
  Branches       6523     6611      +88     
============================================
+ Hits          20621    21188     +567     
+ Misses        18423    18376      -47     
- Partials       1457     1508      +51     

Impacted Files	Coverage Δ	Complexity Δ
...e/gsi/dataset/spi/financial/api/attrs/TypeKey.java	76.47% <76.47%> (ø)	7.00 <7.00> (?)
...rc/main/java/de/gsi/dataset/utils/StreamUtils.java	78.26% <78.26%> (ø)	6.00 <6.00> (?)
.../chart/renderer/spi/financial/HighLowRenderer.java	82.75% <82.75%> (ø)	18.00 <18.00> (?)
...i/financial/css/FinancialColorSchemeConstants.java	83.33% <83.33%> (ø)	2.00 <2.00> (?)
...rt/renderer/spi/financial/CandleStickRenderer.java	86.92% <86.92%> (ø)	30.00 <30.00> (?)
...derer/spi/financial/AbstractFinancialRenderer.java	89.47% <89.47%> (ø)	7.00 <7.00> (?)
...ava/de/gsi/dataset/spi/financial/OhlcvDataSet.java	90.32% <90.32%> (ø)	20.00 <20.00> (?)
.../spi/financial/css/FinancialColorSchemeConfig.java	90.47% <90.47%> (ø)	25.00 <25.00> (?)
...ataset/spi/financial/api/attrs/AttributeModel.java	96.66% <96.66%> (ø)	27.00 <27.00> (?)
...hartfx-chart/src/main/java/de/gsi/chart/Chart.java	79.59% <100.00%> (+1.70%)	109.00 <1.00> (+5.00)
... and 24 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 8caa280...b560516. Read the comment docs.

[RalphSteinhagen]

Very nice pull-request and only minor comments that should be addressed, mainly:

a) to open/keep the candle-stick and hi-lo renderer also compatible with other more generic DataSet implementation (e.g. MultiDimDoubleDataSet or any other DataSet with nDim>=6)

b) you may consider adding some JavaDoc and '@author' and references to the classes as documentation, acknowledgement of your work, and so that everyone knows long-term who has produced these nice renderer additions.

In any case, very nice PR and if you agree to the minor changes mentioned above, I'd approve and sponsor this PR and long-term maintenance within chart-fx.

[wirew0rm]

Hey there,

first of all also from me a big thank you for your contribution, this all looks very nice. It looks like there's only a few small things left before this can be merged.

I think Ralph already pointed out the bigger API ones, so mine are mostly smaller nits and suggestions. Please feel free to disagree with any of them, the only thing that should really be changed before mege is the number of dimensions for the OhlcvDataSet (in addition to Ralphs points).

I also like the samples, if you want you can also add some nice screenshots into doc/pics and add them to the gallery in the Readme.md, otherwise we will do that after the merge is done.

[raven2cz]

The review impl. changes are pushed now. I will see how about restyle again....after few next commits.....
Pictures are added too. Some javadoc too. Additional features will be part of next PR.

[raven2cz]

Done

[RalphSteinhagen]

@raven2cz two quick questions before merging:

In the renderer you wrote:

if (ds.getDimension() > 7)
                continue;

shouldn't be this

if (ds.getDimension() < 7)
                continue;

ie. that any DataSet that has less than 7 dimensions is rejected? The way '> 7 ' would reject only DataSets with dim = 8, 9, 10, ....

Second questions: do you want the commit structure preserved or can we squash-and-merge your PR?

Thanks again for your patience and for the follow-up commits. I feel this is a very nice additions and would be happy to merge if you are OK with this as well.

[wirew0rm]

Changes looks good, thanks also for the feedback on the more open points and the screenshots.

I think that one compare operation should work the other way round, but then we're good to go.

EDIT: seems like I somehow missed Ralph's comment :/

[wirew0rm]

Very nice, lets get this merged 🚀

[RalphSteinhagen]

LGTM!