Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replaced relevant ChartUtils methods with Double extensions #2994

Merged
merged 8 commits into from
Dec 28, 2017
Merged

Replaced relevant ChartUtils methods with Double extensions #2994

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5dd1091
Replaced relevant `ChartUtils` methods with `Double` extensions
jjatie Nov 13, 2017
593e73c
Updated `ChartUtilsTests` to match changes
jjatie Nov 14, 2017
23aa2f2
Merge branch 'master' into double-extensions
jjatie Nov 15, 2017
54440e1
Merge remote-tracking branch 'upstream/master' into double-extensions
jjatie Nov 15, 2017
7a34187
Pulled latest master
jjatie Nov 15, 2017
a9e72c1
Merge remote-tracking branch 'origin/double-extensions' into double-e…
jjatie Nov 15, 2017
6a57a6a
Merge remote-tracking branch 'upstream/master' into double-extensions
jjatie Dec 11, 2017
672f25e
Pulled latest master
jjatie Dec 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Source/Charts/Charts/ChartViewBase.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ open class ChartViewBase: NSUIView, ChartDataProvider, AnimatorDelegate
if _defaultValueFormatter is DefaultValueFormatter
{
// setup the formatter with a new number of digits
let digits = ChartUtils.decimals(reference)
let digits = reference.decimalPlaces
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm no native speaker. is decimalPlaces ok? @petester42 @danielgindi


(_defaultValueFormatter as? DefaultValueFormatter)?.decimals
= digits
Expand Down
6 changes: 3 additions & 3 deletions Source/Charts/Renderers/AxisRendererBase.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ open class AxisRendererBase: Renderer

// Find out how much spacing (in y value space) between axis values
let rawInterval = range / Double(labelCount)
var interval = ChartUtils.roundToNextSignificant(number: Double(rawInterval))
var interval = rawInterval.roundedToNextSignficant()

// If granularity is enabled, then do not allow the interval to go below specified granularity.
// This is used to avoid repeated values when rounding values for display.
Expand All @@ -122,7 +122,7 @@ open class AxisRendererBase: Renderer
}

// Normalize interval
let intervalMagnitude = ChartUtils.roundToNextSignificant(number: pow(10.0, Double(Int(log10(interval)))))
let intervalMagnitude = pow(10.0, Double(Int(log10(interval)))).roundedToNextSignficant()
let intervalSigDigit = Int(interval / intervalMagnitude)
if intervalSigDigit > 5
{
Expand Down Expand Up @@ -162,7 +162,7 @@ open class AxisRendererBase: Renderer
first -= interval
}

let last = interval == 0.0 ? 0.0 : ChartUtils.nextUp(floor(yMax / interval) * interval)
let last = interval == 0.0 ? 0.0 : (floor(yMax / interval) * interval).nextUp

if interval != 0.0 && last != first
{
Expand Down
6 changes: 3 additions & 3 deletions Source/Charts/Renderers/YAxisRendererRadarChart.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ open class YAxisRendererRadarChart: YAxisRenderer

// Find out how much spacing (in yValue space) between axis values
let rawInterval = range / Double(labelCount)
var interval = ChartUtils.roundToNextSignificant(number: Double(rawInterval))
var interval = rawInterval.roundedToNextSignficant()

// If granularity is enabled, then do not allow the interval to go below specified granularity.
// This is used to avoid repeated values when rounding values for display.
Expand All @@ -55,7 +55,7 @@ open class YAxisRendererRadarChart: YAxisRenderer
}

// Normalize interval
let intervalMagnitude = ChartUtils.roundToNextSignificant(number: pow(10.0, floor(log10(interval))))
let intervalMagnitude = pow(10.0, floor(log10(interval))).roundedToNextSignficant()
let intervalSigDigit = Int(interval / intervalMagnitude)

if intervalSigDigit > 5
Expand Down Expand Up @@ -98,7 +98,7 @@ open class YAxisRendererRadarChart: YAxisRenderer
first -= interval
}

let last = interval == 0.0 ? 0.0 : ChartUtils.nextUp(floor(yMax / interval) * interval)
let last = interval == 0.0 ? 0.0 : (floor(yMax / interval) * interval).nextUp

if interval != 0.0
{
Expand Down
79 changes: 35 additions & 44 deletions Source/Charts/Utils/ChartUtils.swift
100755 → 100644
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,40 @@ extension CGSize {
}
}

extension Double {
/// Rounds the number to the nearest multiple of it's order of magnitude, rounding away from zero if halfway.
func roundedToNextSignficant() -> Double {
guard
!isInfinite,
!isNaN,
self != 0
else { return self }

let d = ceil(log10(self < 0 ? -self : self))
let pw = 1 - Int(d)
let magnitude = pow(10.0, Double(pw))
let shifted = (self * magnitude).rounded()
return shifted / magnitude
}

var decimalPlaces: Int {
guard
!isNaN,
!isInfinite,
self != 0.0
else { return 0 }

let i = self.roundedToNextSignficant()

guard
!i.isInfinite,
!i.isNaN
else { return 0 }

return Int(ceil(-log10(i))) + 2
}
}

open class ChartUtils
{
fileprivate static var _defaultValueFormatter: IValueFormatter = ChartUtils.generateDefaultValueFormatter()
Expand All @@ -41,50 +75,7 @@ open class ChartUtils
internal static let DEG2RAD = Double.pi / 180.0
internal static let RAD2DEG = 180.0 / Double.pi
}

internal class func roundToNextSignificant(number: Double) -> Double
{
if number.isInfinite || number.isNaN || number == 0
{
return number
}

let d = ceil(log10(number < 0.0 ? -number : number))
let pw = 1 - Int(d)
let magnitude = pow(Double(10.0), Double(pw))
let shifted = round(number * magnitude)
return shifted / magnitude
}

internal class func decimals(_ number: Double) -> Int
{
if number.isNaN || number.isInfinite || number == 0.0
{
return 0
}

let i = roundToNextSignificant(number: Double(number))

if i.isInfinite || i.isNaN
{
return 0
}

return Int(ceil(-log10(i))) + 2
}

internal class func nextUp(_ number: Double) -> Double
Copy link
Member

@liuxuan30 liuxuan30 Dec 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swift doc:

For any finite value x, x.nextUp is greater than x. For nan or infinity, x.nextUp is x itself. The following special cases also apply:

If x is -infinity, then x.nextUp is -greatestFiniteMagnitude.
If x is -leastNonzeroMagnitude, then x.nextUp is -0.0.
If x is zero, then x.nextUp is leastNonzeroMagnitude.
If x is greatestFiniteMagnitude, then x.nextUp is infinity.

I'm kind of confused If x is -infinity, then x.nextUp is -greatestFiniteMagnitude. because For nan or infinity, x.nextUp is x itself, and infinity is not greatestFiniteMagnitude? last one is "If x is greatestFiniteMagnitude, then x.nextUp is infinity."

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only place this is used is in the axis renderers. I removed this implementation because the default one is safer to pass into the call sites.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep but it's different than the library implementation, towards the infinity check and result. I need to check several places.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the real issue is swift doc has paradox:

For nan or infinity, x.nextUp is x itself. The following special cases also apply

does infinity here contains both +inf and -inf? or just +inf?

because it added at last line:

If x is -infinity, then x.nextUp is -greatestFiniteMagnitude

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.infinitiy != -.infinity
It makes sense that the nextUp from negative infinity is the negative greatest finite magnitude. The opposite is true for nextDown()

{
if number.isInfinite || number.isNaN
{
return number
}
else
{
return number + Double.ulpOfOne
}
}


/// Calculates the position around a center point, depending on the distance from the center, and the angle of the position around the center.
internal class func getPosition(center: CGPoint, dist: CGFloat, angle: CGFloat) -> CGPoint
{
Expand Down
12 changes: 6 additions & 6 deletions Tests/Charts/ChartUtilsTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class ChartUtilsTests: XCTestCase {

let number = Double.nan

let actual = ChartUtils.decimals(number)
let actual = number.decimalPlaces
let expected = 0

XCTAssertEqual(expected, actual)
Expand All @@ -27,7 +27,7 @@ class ChartUtilsTests: XCTestCase {

let number = Double.infinity

let actual = ChartUtils.decimals(number)
let actual = number.decimalPlaces
let expected = 0

XCTAssertEqual(expected, actual)
Expand All @@ -37,7 +37,7 @@ class ChartUtilsTests: XCTestCase {

let number = 0.0

let actual = ChartUtils.decimals(number)
let actual = number.decimalPlaces
let expected = 0

XCTAssertEqual(expected, actual)
Expand All @@ -47,7 +47,7 @@ class ChartUtilsTests: XCTestCase {

let number = Double.greatestFiniteMagnitude

let actual = ChartUtils.decimals(number)
let actual = number.decimalPlaces
let expected = 0

XCTAssertEqual(expected, actual)
Expand All @@ -57,7 +57,7 @@ class ChartUtilsTests: XCTestCase {

let number = Double.leastNormalMagnitude

let actual = ChartUtils.decimals(number)
let actual = number.decimalPlaces
let expected = 310 // Don't think this is supposed to be this value maybe 0?

XCTAssertEqual(expected, actual)
Expand All @@ -67,7 +67,7 @@ class ChartUtilsTests: XCTestCase {

let number = 13.123123

let actual = ChartUtils.decimals(number)
let actual = number.decimalPlaces
let expected = 1 // Don't think this is supposed to be this value maybe 6?

XCTAssertEqual(expected, actual)
Expand Down