Financial Charts - realtime processing, re-sample live demos, improved renderers, navigation etc. #326

raven2cz · 2020-12-05T22:52:56Z

Add next features and improvements for financial charts.

Tick OHLC processing demo with re-sample support. Real-time processing.
Improved renderers - better calculation of bars distances, support for y-range min-max feature.
Better navigation for financial charts, can be tested in new sample for real-time processing.
YWatchValueIndicator for better visualization of y-values and auto handling by close prices. Add to the same sample as before.
OHLC domain objects prepared for volume profiles, range bars, foot prints, market profiles (domain objects are part of this commit).
Better css styling prepared. Usage for value and range indicators for Support/Resistance levels. Add support for financial themes.

…d renderers.

…d renderers finished.

…ntation added.

codecov · 2020-12-05T23:00:35Z

Codecov Report

Merging #326 (33babea) into master (20ae707) will increase coverage by 0.24%.
The diff coverage is 85.15%.

@@             Coverage Diff              @@
##             master     #326      +/-   ##
============================================
+ Coverage     51.59%   51.83%   +0.24%     
- Complexity     7266     7313      +47     
============================================
  Files           393      394       +1     
  Lines         41072    41189     +117     
  Branches       6611     6627      +16     
============================================
+ Hits          21190    21351     +161     
+ Misses        18376    18333      -43     
+ Partials       1506     1505       -1

Impacted Files	Coverage Δ	Complexity Δ
...rt/renderer/spi/financial/CandleStickRenderer.java	`89.67% <50.00%> (+2.74%)`	`31.00 <0.00> (+1.00)`
.../chart/renderer/spi/financial/HighLowRenderer.java	`86.39% <50.00%> (+3.63%)`	`20.00 <0.00> (+2.00)`
...derer/spi/financial/AbstractFinancialRenderer.java	`88.15% <83.67%> (-1.32%)`	`16.00 <9.00> (+9.00)`	⬇️
...ava/de/gsi/chart/plugins/YWatchValueIndicator.java	`87.32% <87.32%> (ø)`	`16.00 <16.00> (?)`
.../spi/financial/css/FinancialColorSchemeConfig.java	`90.90% <100.00%> (+0.43%)`	`26.00 <0.00> (+1.00)`
...java/de/gsi/chart/axes/spi/DefaultNumericAxis.java	`71.48% <0.00%> (-2.48%)`	`66.00% <0.00%> (-2.00%)`
.../main/java/de/gsi/chart/axes/spi/AbstractAxis.java	`76.26% <0.00%> (-0.15%)`	`182.00% <0.00%> (-1.00%)`
...hart/plugins/measurements/DataSetMeasurements.java	`75.77% <0.00%> (+0.34%)`	`153.00% <0.00%> (+2.00%)`
...n/java/de/gsi/chart/renderer/spi/GridRenderer.java	`81.94% <0.00%> (+0.92%)`	`47.00% <0.00%> (+1.00%)`
... and 7 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 20ae707...33babea. Read the comment docs.

lgtm-com · 2020-12-05T23:07:54Z

This pull request introduces 1 alert when merging 57361ae into 20ae707 - view on LGTM.com

new alerts:

1 for Potential input resource leak

lgtm-com · 2020-12-06T08:41:23Z

This pull request introduces 1 alert when merging 2130e88 into 20ae707 - view on LGTM.com

new alerts:

1 for Potential input resource leak

RalphSteinhagen

very nice PR, only minor things.
The main questions/comments:

new interface -> is this really necessary?
authorship (not critical) -> feel free to claim your @author JavaDoc tags.
See other comments/questions for details.

...-samples/src/main/java/de/gsi/chart/samples/financial/AbstractBasicFinancialApplication.java

RalphSteinhagen · 2020-12-07T09:32:15Z

chartfx-samples/src/main/java/de/gsi/chart/samples/financial/service/SCIDByNio.java

+        FileInputStream fis = new FileInputStream(f);
+
+        //----------------------------------
+        fileChannel = fis.getChannel();


potential resource leak fis being opened in this function and not clear that it is closed elsewhere. Try-with-resources and auto-close wouldn't probably work here.... 🤔

Since this is an example, it's not that critical but would be -- in case this is a false positive -- to suppress the LGTM warning.

Yes, it cannot be closed in the same method because it is a opened stream for ohlc realtime processing. There is another method for closing stream in the finished/exit parts of sample demo. LGTM cannot recognize this handling by some simple checking of try-resource. The LGTM suppress tags are already added.

RalphSteinhagen · 2020-12-07T09:36:48Z

chartfx-chart/src/main/java/de/gsi/chart/plugins/YWatchValueIndicator.java

@@ -0,0 +1,135 @@
+/**


maybe add your @author tag below and reference to CERN behind the original primary author

super, changed according your proposal.

RalphSteinhagen · 2020-12-07T09:45:10Z

chartfx-dataset/src/main/java/de/gsi/dataset/spi/financial/OhlcvChangeListener.java

+/**
+ * OHLCV Listener about structure changes.
+ */
+public interface OhlcvChangeListener {


Is this new interface really necessary? As an alternative: would it be an option to derive a new event type from 'UpdateEvente.g.IOhlEvent' and -- as an API contract -- to put the IOhlcvItem to the event's payload? The rationale is to minimise the number of public API/interface definitions that need to be maintained long-term... in any case, am open for discussion.

This is perfect, reviews are very important. Thanks! Removed. Not necessary. Refactored for an internal usage in the example only!

chartfx-chart/src/main/resources/de/gsi/chart/financial/chart.css

lgtm-com · 2020-12-08T12:37:21Z

This pull request introduces 1 alert when merging 33babea into 20ae707 - view on LGTM.com

new alerts:

1 for Potential input resource leak

RalphSteinhagen

Thanks for the modifications and comments. If you are OK with this, I'd squash-merge it into master. We would do a micro-release in the coming days...

raven2cz · 2020-12-08T13:32:10Z

Thanks for the modifications and comments. If you are OK with this, I'd squash-merge it into master. We would do a micro-release in the coming days...

Thank you. Yes, squash-merge.
Next steps, before next fin.charts, there is necessary renderer / visualization of orders and positions entry/exits in the actual charts, and maybe by table way too. I see that some renderers support table structure too. These orders are not large implementation, but there has to be defined correctly several attributes (cca 15-20 atrtrs) for each executed position. So, graphic is not complicated, but domain object includes few attributes which have to be presented by positions in the chart or alternative table view (optional).

wirew0rm · 2020-12-08T18:00:56Z

Hey, didn't hava a chance to look at it in detail, but what I looked at looked nice 👍

Regarding tables, this is not done as a renderer, but as a dedicated plugin (TableViewer for just showing data as a table and EditDataSet for editing points). As it was developed before the DataSet Interface was extended to multiple dimensions, it currently only supports X/Y data. It would be nice to extend it to multidimensional datasets and to take the label/unit information from the axisDescriptions into account.

RalphSteinhagen · 2020-12-09T08:01:01Z

@raven2cz just as a note/addition to what @wirew0rm mentioned:
It's equally possible to implement it as a Renderer.

The interface

List<DataSet> render(GraphicsContext gc, Chart chart, int dataSetOffset, ObservableList<DataSet> datasets);

provides access to the Canvas (via GraphicsContext) as well as the corresponding Chart and all of its sub-component including notably the Pane getPlotForeground() which is drawn on top of the chart and could hold also a custom rendered table component.

The primary reasons why we (originally) implemented it as a 'plugin' was to adorn the chart and to allow the user to quickly switch the table 'on' or 'off' while still relying on the same generic DataSet that is drawn in the background and also because we didn't have the DataViewer functionality yet at that time.

Using the DataViewer, this could be also implemented externally by switching between two DataViews, one with the chart, and the other one holding the table. This would also correspond to a more classic JavaFX/UI design.

N.B. the latter option should be also more performant since it removes/adds the chart from the JavaFX scenegraph when needed which minimises unnecessary draw routines (if you have a lot of data points). The Renderer/ChartPlugin option has the advantage that one does not have to synchronise the attached DataSets between two Pane components.

Maybe have a look at the DataViewer and let us know your view/angle. :-)

raven2cz added 5 commits November 28, 2020 14:10

Financial Charts - realtime processing, re-sample live demos, improve…

29ebbcd

…d renderers.

Value and range indicator testing for financial charts.

7dfd8e6

Financial Charts - realtime processing, re-sample live demos, improve…

12692f8

…d renderers finished.

Financial Charts - realtime processing, re-sample live demos - docume…

ce89314

…ntation added.

Merge remote-tracking branch 'upstream/master'

57361ae

restyled-io bot mentioned this pull request Dec 5, 2020

Restyle Financial Charts - realtime processing, re-sample live demos, improved renderers, navigation etc. #327

Closed

raven2cz added 2 commits December 6, 2020 09:15

Restyled changes phase 1.

abdc281

Restyled changes phase 2.

2130e88

raven2cz marked this pull request as ready for review December 6, 2020 09:24

RalphSteinhagen self-requested a review December 7, 2020 09:08

RalphSteinhagen reviewed Dec 7, 2020

View reviewed changes

Review changes implemented.

33babea

RalphSteinhagen approved these changes Dec 8, 2020

View reviewed changes

RalphSteinhagen merged commit c0ea477 into fair-acc:master Dec 8, 2020

wirew0rm mentioned this pull request Dec 16, 2020

maven: set maven surefire locale to en_US #331

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Financial Charts - realtime processing, re-sample live demos, improved renderers, navigation etc. #326

Financial Charts - realtime processing, re-sample live demos, improved renderers, navigation etc. #326

raven2cz commented Dec 5, 2020

codecov bot commented Dec 5, 2020 •

edited

Loading

lgtm-com bot commented Dec 5, 2020

lgtm-com bot commented Dec 6, 2020

RalphSteinhagen left a comment

RalphSteinhagen Dec 7, 2020

raven2cz Dec 8, 2020

RalphSteinhagen Dec 7, 2020

raven2cz Dec 8, 2020

RalphSteinhagen Dec 7, 2020

raven2cz Dec 8, 2020

lgtm-com bot commented Dec 8, 2020

RalphSteinhagen left a comment

raven2cz commented Dec 8, 2020

wirew0rm commented Dec 8, 2020

RalphSteinhagen commented Dec 9, 2020

Financial Charts - realtime processing, re-sample live demos, improved renderers, navigation etc. #326

Financial Charts - realtime processing, re-sample live demos, improved renderers, navigation etc. #326

Conversation

raven2cz commented Dec 5, 2020

codecov bot commented Dec 5, 2020 • edited Loading

Codecov Report

lgtm-com bot commented Dec 5, 2020

lgtm-com bot commented Dec 6, 2020

RalphSteinhagen left a comment

Choose a reason for hiding this comment

RalphSteinhagen Dec 7, 2020

Choose a reason for hiding this comment

raven2cz Dec 8, 2020

Choose a reason for hiding this comment

RalphSteinhagen Dec 7, 2020

Choose a reason for hiding this comment

raven2cz Dec 8, 2020

Choose a reason for hiding this comment

RalphSteinhagen Dec 7, 2020

Choose a reason for hiding this comment

raven2cz Dec 8, 2020

Choose a reason for hiding this comment

lgtm-com bot commented Dec 8, 2020

RalphSteinhagen left a comment

Choose a reason for hiding this comment

raven2cz commented Dec 8, 2020

wirew0rm commented Dec 8, 2020

RalphSteinhagen commented Dec 9, 2020

codecov bot commented Dec 5, 2020 •

edited

Loading