Axis Renderers Cleanup #3164

jjatie · 2018-01-09T00:32:34Z

Replaced custom algorithms with built-in ones
Made axis renderer implementations feel "Swift-ier"

codecov-io · 2018-01-09T02:36:00Z

Codecov Report

Merging #3164 into 4.0.0 will increase coverage by 0.12%.
The diff coverage is 28.91%.

@@            Coverage Diff             @@
##            4.0.0    #3164      +/-   ##
==========================================
+ Coverage   31.36%   31.48%   +0.12%     
==========================================
  Files         112      112              
  Lines       10246    10051     -195     
==========================================
- Hits         3214     3165      -49     
+ Misses       7032     6886     -146

Impacted Files	Coverage Δ
Source/Charts/Highlight/ChartHighlighter.swift	`28.57% <ø> (ø)`	⬆️
Source/Charts/Charts/BarLineChartViewBase.swift	`66.01% <ø> (ø)`	⬆️
...s/Data/Implementations/Standard/ChartDataSet.swift	`25.92% <ø> (-2.65%)`	⬇️
...ts/Renderers/YAxisRendererHorizontalBarChart.swift	`0% <0%> (ø)`	⬆️
...ts/Renderers/XAxisRendererHorizontalBarChart.swift	`0% <0%> (ø)`	⬆️
...arts/Data/Implementations/Standard/ChartData.swift	`19.76% <0%> (+4.76%)`	⬆️
...rce/Charts/Renderers/XAxisRendererRadarChart.swift	`0% <0%> (ø)`	⬆️
...rce/Charts/Renderers/YAxisRendererRadarChart.swift	`0% <0%> (ø)`	⬆️
Source/Charts/Renderers/YAxisRenderer.swift	`59.07% <42.52%> (+0.19%)`	⬆️
Source/Charts/Renderers/LineChartRenderer.swift	`57.18% <50%> (ø)`	⬆️
... and 3 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 be8b386...6d8471d. Read the comment docs.

liuxuan30 · 2018-05-07T02:24:16Z

Source/Charts/Data/Implementations/Standard/BarChartDataSet.swift

-                _entryCountStacks += 1
-            }
-        }
+        entries.forEach { _entryCountStacks += $0.yValues?.count ?? 1 }


I saw you merged data set cleanup PR, but I still seeing many diffs about data set that has been merged? Could you help check what's wrong?

liuxuan30 · 2018-05-17T00:50:29Z

@jjatie busy recently? I will look at other PRs that are not in any projects and try merge some before having your reply

jjatie · 2018-05-17T10:30:46Z

Yes, very busy lately. Will hopefully get to it this weekend

liuxuan30 · 2018-05-18T00:28:52Z

take your time, no hurry :)

jjatie · 2018-09-17T13:52:45Z

@liuxuan30 Please review!

liuxuan30 · 2018-09-19T07:15:56Z

ooo. Finally free from work overhead? :)

I will try after cleaning the inbox

jjatie · 2018-09-19T10:39:26Z

@liuxuan30 Haha, not yet! Just needed a break. I'm still very short handed; it's hard to find iOS devs.

liuxuan30 · 2018-09-20T01:16:09Z

hahah. my neighbor team is totally gone so I took over their app. And they are using Charts.. so maybe I can allocate more daytime on this :)

jjatie · 2018-12-11T11:12:46Z

@liuxuan30 Can you take a look at this soon? (I'll fix the .swift-version after you review)

liuxuan30 · 2018-12-17T01:21:13Z

hmm.. is this top priority? I was thinking looking at the line sharp edge and rounded bar this week
I'm busy with a POC last week, but this week should be easier for me.

If this PR is not very hard to review, I think we can finish this PR this week.

Source/Charts/Data/Implementations/Standard/ChartData.swift

Source/Charts/Data/Implementations/Standard/ChartDataSet.swift

Source/Charts/Renderers/LineChartRenderer.swift

Source/Charts/Renderers/XAxisRenderer.swift

Source/Charts/Renderers/XAxisRendererHorizontalBarChart.swift

Source/Charts/Renderers/XAxisRenderer.swift

Source/Charts/Renderers/YAxisRendererHorizontalBarChart.swift

Source/Charts/Renderers/YAxisRendererRadarChart.swift

liuxuan30 · 2018-12-17T09:00:43Z

finished the 1st round review. Some questions may seem silly :( a bit rusty for swift

Added TODOs for areas where simple changes can help improve Swift consistency.

MutableCollection RandomAccessCollection RangeReplaceableCollection

Refactored use of `ChartData` to use new `Collection` conformances

to take advantage of collection conformance.

Replaced custom algorithms with built-in ones Made axis renderer implementations feel "Swift-ier"

Also added remove subrange.

Removed redundancy from min/max logic. Lots of naming changes. Cleaner implementations.

Source/Charts/Renderers/XAxisRenderer.swift

liuxuan30 · 2019-01-03T01:52:15Z

@jjatie please take a look above unresolved sessions. once it's done I think we are good to go

liuxuan30 · 2019-01-04T02:46:52Z

merged. let's go next one

SergiuCoca · 2019-02-19T14:28:43Z

Hi @liuxuan30 ,

I'm using the new released version (3.2.2) and it seems that #3798 issue isn't fixed. As I saw the changes were made in this PR. I looked into LineChartRender.swift and I don't have the changes even if I updated the library. Do you know why? Any solution for this?

Thank you!

jjatie · 2019-02-21T01:01:27Z

@SergiuCoca This was merged into the 4.0.0 branch

jjatie added this to the 4.0.0 milestone Jan 9, 2018

jjatie changed the title ~~Swift-ified and Unified Style~~ Axis Renderers Cleanup Jan 9, 2018

jjatie mentioned this pull request Jan 16, 2018

Axis cleanup #3187

Closed

liuxuan30 reviewed May 7, 2018

View reviewed changes

liuxuan30 mentioned this pull request Jul 24, 2018

Changes for Swift 4.2, Xcode 10 and iOS 12 #3522

Merged

liuxuan30 reviewed Dec 17, 2018

View reviewed changes

Source/Charts/Data/Implementations/Standard/ChartData.swift Show resolved Hide resolved

Source/Charts/Data/Implementations/Standard/ChartData.swift Show resolved Hide resolved

liuxuan30 reviewed Dec 17, 2018

View reviewed changes

jjatie added 9 commits December 25, 2018 01:15

Cleaned up ChartDataSet logic

aa7e20a

Added TODOs for areas where simple changes can help improve Swift consistency.

Added Collection conformances

438dae7

MutableCollection RandomAccessCollection RangeReplaceableCollection

[#3018]

8aba022

Refactored use of `ChartData` to use new `Collection` conformances

Fixed required initializers

2ced54f

ChartData adopts ExressibleByArrayLiteral

7dfb59e

Modified demos

b554fb0

to take advantage of collection conformance.

Pulled latest master

51501a1

Unified Style

78fb682

Replaced custom algorithms with built-in ones Made axis renderer implementations feel "Swift-ier"

Updates for PR

bdf6f6d

Also added remove subrange.

jjatie added 10 commits December 25, 2018 01:15

Refactored ChartData

da2ff2d

Removed redundancy from min/max logic. Lots of naming changes. Cleaner implementations.

Fixed horizontal barchart bug,

9aaee13

Removed unnecessary get from subscripts.

f399bca

Disabled remove(at:) for CombinedChartView

bd1bc8f

Relocated appendEntry(_:todataSet:)

74f28b1

Disabled Collection support for CombinedChartData

aec150e

Removed used of dataSet(forIndex:)

f606175

Fixed merge conflicts

6db01d2

updated demos

b8aa08e

PR Fixes

6d8471d

liuxuan30 reviewed Dec 26, 2018

View reviewed changes

Source/Charts/Renderers/XAxisRenderer.swift Show resolved Hide resolved

Source/Charts/Renderers/XAxisRenderer.swift Outdated Show resolved Hide resolved

liuxuan30 mentioned this pull request Dec 26, 2018

Performance issue: drawCircles(context: CGContext) in LineChart #3798

Closed

Fixed axisLabels calculation

52480db

liuxuan30 merged commit 5689a1c into ChartsOrg:4.0.0 Jan 4, 2019

mock123 mentioned this pull request Feb 17, 2019

No circle is drawn on a line chart when there is only one data. (version 3.2.2 only) #3848

Closed

jjatie deleted the 4.0/axis-renderer-cleanup branch February 21, 2019 01:01

caloon mentioned this pull request Sep 3, 2019

Stronger Testing Procedures #4132

Open

liuxuan30 mentioned this pull request Sep 26, 2019

EXC_BREAKPOINT on LineChartRenderer.drawCircles #4049

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Axis Renderers Cleanup #3164

Axis Renderers Cleanup #3164

jjatie commented Jan 9, 2018

codecov-io commented Jan 9, 2018 •

edited

Loading

liuxuan30 May 7, 2018 •

edited

Loading

liuxuan30 May 15, 2018

liuxuan30 commented May 17, 2018 •

edited

Loading

jjatie commented May 17, 2018

liuxuan30 commented May 18, 2018

jjatie commented Sep 17, 2018

liuxuan30 commented Sep 19, 2018 •

edited

Loading

jjatie commented Sep 19, 2018 •

edited

Loading

liuxuan30 commented Sep 20, 2018

jjatie commented Dec 11, 2018

liuxuan30 commented Dec 17, 2018 •

edited

Loading

liuxuan30 commented Dec 17, 2018

liuxuan30 commented Jan 3, 2019

liuxuan30 commented Jan 4, 2019

SergiuCoca commented Feb 19, 2019

jjatie commented Feb 21, 2019

Axis Renderers Cleanup #3164

Axis Renderers Cleanup #3164

Conversation

jjatie commented Jan 9, 2018

codecov-io commented Jan 9, 2018 • edited Loading

Codecov Report

liuxuan30 May 7, 2018 • edited Loading

Choose a reason for hiding this comment

liuxuan30 May 15, 2018

Choose a reason for hiding this comment

liuxuan30 commented May 17, 2018 • edited Loading

jjatie commented May 17, 2018

liuxuan30 commented May 18, 2018

jjatie commented Sep 17, 2018

liuxuan30 commented Sep 19, 2018 • edited Loading

jjatie commented Sep 19, 2018 • edited Loading

liuxuan30 commented Sep 20, 2018

jjatie commented Dec 11, 2018

liuxuan30 commented Dec 17, 2018 • edited Loading

liuxuan30 commented Dec 17, 2018

liuxuan30 commented Jan 3, 2019

liuxuan30 commented Jan 4, 2019

SergiuCoca commented Feb 19, 2019

jjatie commented Feb 21, 2019

codecov-io commented Jan 9, 2018 •

edited

Loading

liuxuan30 May 7, 2018 •

edited

Loading

liuxuan30 commented May 17, 2018 •

edited

Loading

liuxuan30 commented Sep 19, 2018 •

edited

Loading

jjatie commented Sep 19, 2018 •

edited

Loading

liuxuan30 commented Dec 17, 2018 •

edited

Loading