ChartRenderer's must be initialized with a chart

[jjatie]

No description provided.

[codecov-io]

Codecov Report

Merging #2982 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2982   +/-   ##
=======================================
  Coverage   19.64%   19.64%           
=======================================
  Files         113      113           
  Lines       13508    13508           
=======================================
  Hits         2653     2653           
  Misses      10855    10855

Impacted Files	Coverage Δ
...ce/Charts/Renderers/CandleStickChartRenderer.swift	0% <ø> (ø)	⬆️
Source/Charts/Renderers/LineChartRenderer.swift	55.3% <ø> (ø)	⬆️
...rce/Charts/Renderers/YAxisRendererRadarChart.swift	0% <ø> (ø)	⬆️
...ource/Charts/Renderers/CombinedChartRenderer.swift	0% <ø> (ø)	⬆️
Source/Charts/Renderers/PieChartRenderer.swift	0% <ø> (ø)	⬆️
...ts/Renderers/XAxisRendererHorizontalBarChart.swift	0% <ø> (ø)	⬆️
Source/Charts/Renderers/ScatterChartRenderer.swift	0% <ø> (ø)	⬆️
Source/Charts/Renderers/BarChartRenderer.swift	41.37% <ø> (ø)	⬆️
.../Charts/Renderers/HorizontalBarChartRenderer.swift	0% <ø> (ø)	⬆️
Source/Charts/Renderers/BubbleChartRenderer.swift	0% <ø> (ø)	⬆️
... and 2 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 fb02977...e458bd9. Read the comment docs.

[liuxuan30]

What's the reason for this?

[jjatie]

Does it make sense that a ChartRenderer has no chart to render?

My guess is this initializer parameter was made optional because the weak property declaration forced the property to be declared as optional.

[liuxuan30]

@objc open weak var dataProvider: BarChartDataProvider? is declared as weak, sounds like a historical reason or habit to use optional for params.

This PR is in conjunction with #2981 to make animator non nil as well.

Looks fine to merge. I will take a deeper look at both PRs and original code.

BTW @jjatie among your PRs, is there any dependency/order to merge? I saw these two PRs are straightforward and can pass CI as the main reason to merge in short time.

[liuxuan30]

CombinedChartRenderer also has similar footprint,

@objc public init(chart: CombinedChartView?, animator: Animator, viewPortHandler: ViewPortHandler?)

shall we change it as well?

[jjatie]

Oops, I missed that one. Fixing now.

[jjatie]

Any PR that has a dependency on another as a comment indicating that. For the most part there are no dependencies.

[liuxuan30]

oops, conflicts. when you solve I will check again.
I think we need a way to reduce this kind of conflicts, it causes more work for you @jjatie. One PR merged and similar ones conflicted.

[jjatie]

@liuxuan30 Fixed, but pull #2980 first. While in some cases I might be able to consolidate some PRs, I think for the most part they should be separate.

[liuxuan30]

this can be next to merge? have to work then. check back later

[jjatie]

Yes. It's fixed

[liuxuan30]

squash to merge for this PR