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.

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

Implement Trendline for iOS #3272

Open
wants to merge 2 commits into
base: ucr
Choose a base branch
from
Open

Conversation

daki7711
Copy link
Contributor

@daki7711 daki7711 commented Dec 2, 2024

-What does this PR accomplish?

Adds the trendline component for ios

daki7711 and others added 2 commits December 2, 2024 09:55
Change-Id: I335fea5906375f6e65993b69c2d729bbd4aadc8c
Change-Id: Ia2324a0800ced921c2622d1329485f1541331eab
Comment on lines +1 to +7
//
// ChartComponent.swift
// AIComponentKit
//
// Created by David Kim on 3/19/24.
// Copyright © 2024 Massachusetts Institute of Technology. All rights reserved.
//
Copy link
Member

Choose a reason for hiding this comment

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

Correct the header to match the other files, including the license.

Comment on lines +13 to +14
// Method to initialize the chart data object
func initChartData()
Copy link
Member

Choose a reason for hiding this comment

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

Indentation should be +2 here.

Comment on lines +1 to +7
//
// DataCollection.swift
// AIComponentKit
//
// Created by David Kim on 3/28/24.
// Copyright © 2024 Massachusetts Institute of Technology. All rights reserved.
//
Copy link
Member

Choose a reason for hiding this comment

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

Fix header

Comment on lines 1 to 7
//
// DataModel.swift
// AIComponentKit
//
// Created by David Kim on 3/28/24.
// Copyright © 2024 Massachusetts Institute of Technology. All rights reserved.
//
Copy link
Member

Choose a reason for hiding this comment

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

Fix header

Comment on lines +16 to +62
private var _dataFileColumns: [String]?
var dataFileColumns: [String]? {
get { _dataFileColumns }
set { _dataFileColumns = newValue }
}

private var _useSheetHeaders: Bool = false
var useSheetHeaders: Bool {
get { _useSheetHeaders }
set { _useSheetHeaders = newValue }
}

private var _sheetsColumns: [String]?
var sheetsColumns: [String]? {
get { _sheetsColumns }
set { _sheetsColumns = newValue }
}

private var _webColumns: [String]?
var webColumns: [String]? {
get { _webColumns }
set { _webColumns = newValue }
}

private var _dataSourceKey: String?
var dataSourceKey: String? {
get { _dataSourceKey }
set { _dataSourceKey = newValue }
}

private var __elements: String?
var _elements: String? {
get { __elements }
set { __elements = newValue }
}

private var __initialized: Bool = false
var _initialized: Bool {
get { __initialized }
set { __initialized = newValue }
}

private var __tick: Int = 0
var _tick: Int {
get { __tick }
set { __tick = newValue }
}
Copy link
Member

Choose a reason for hiding this comment

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

Note that if these don't need to have any special get/set logic they could simplified to just var foo: type = default

@@ -0,0 +1,86 @@
//
Copy link
Member

Choose a reason for hiding this comment

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

Fix header

Comment on lines +49 to +52
// print(points[0].x)
// if (points[0].x == 1.7976931348623157e+308){
// return
// }
Copy link
Member

Choose a reason for hiding this comment

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

If this code isn't needed please remove it.

Comment on lines +65 to +71
//TODO: how can we resolve this?
// @objc public class func fromUnderlyingValue(_ value: String) -> StrokeStyle? {
// if let intValue = Int(value) {
// return fromUnderlyingValue(intValue)
// }
// return nil
// }
Copy link
Member

Choose a reason for hiding this comment

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

I think for this we will need to have the function take AnyObject and then handle the Int vs String issue internally since Objective-C doesn't have method overloading.

Copy link
Member

Choose a reason for hiding this comment

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

It seems like many of the properties of the Trendline component do not have the correct capitalization. It's not clear that they would be found by the glue code in SchemeKit that connects the block code to the Swift code. Can you double check that the various getter properties are all working correctly?


@objc public var ChartData: ChartData2D {
get {
return _chartData!
Copy link
Member

Choose a reason for hiding this comment

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

Be careful with the forced unwrapping here since if for some reason _chartData is nil when this is called it will crash the app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants