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

Test branch #868

Closed
wants to merge 29 commits into from
Closed

Test branch #868

wants to merge 29 commits into from

Conversation

StephanieB5
Copy link

It's definitely still a work in progress but it maybe a logical place to pull code into the branch before the changes start to affect the renderers. If nothing else the latest commit here has all the previously scattered commits squashed together.

I'm not sure who's notified of pull requests automatically so I'm pinging the same people from the last thread: @liuxuan30, @danielgindi, @RyGuyM

Hope this helps.

StephanieB5 and others added 29 commits January 11, 2016 13:55
This adds a placeholder in the demo app to showcase a line chart with
numeric x values not indices/labels.  Pinging @RyGuyM.
Setting code sign identity values and adding a separate ViewController
in the Demo to showcase and test Time Line Charts.
This change adds a pipeline to get the numeric x values from the
ViewController to the appropriate renderer.  This numeric data will
allow more precise placement of points in graphs.  However, this extra
data isn’t used for rendering yet.
TimeLineChartData is being used.
Since xNumericVal lives in ChartDataEntry it makes sense that Base
Chart Data classes would support xNumericVal as well.  This simplifies
the class hierarchy.  Luckily, scaling is only based on the Y axis and
the x axis is simply cropped.  This means calcMinMax can remain
untouched.
The renderer currently uses _deltaX (to determine max visible range of
x values, etc).  In order to switch to using xNumericVal for charting
the renderer needs to access xNumericValMin/Max for determining deltaX.
In order to use xNumericVal testing for equality of Double will be used
more.  Making this simple test a part of the ChartUtils framework.
This adds a new chart type in the demo app to showcase a line chart with values
on the x-axis represented as Doubles not indices/labels.

- Code sign identity and customizing controller

Set standard generalized code sign identity values

- Added data pipeline for extra numeric data for the X axis

This change adds a pipeline to get the x values as Doubles from the
ViewController to the appropriate renderer.  This extra data will
allow more precise placement of points in graphs.  However, this extra
data isn’t used for rendering yet.

- Put support for xNumericVal to the ChartData classes

The extra data for the x-axis is called xNumericVal.  It lives in ChartDataEntry
to make it available to all chart types.  It makes sense that Base
Chart Data classes would support xNumericVal as well.

The ViewController currently uses _deltaX (to determine max visible range of
x values, etc).  The ViewController needs to access xNumericValMin/Max for
to properly calculate deltaX.

- Add util for testing equality for Doubles

With the addition of Double values on the xAxis testing for equality of Doubles
will be used more.  Making this functionality a part of the ChartUtils framework.
This adds a new chart type, TimeLineChart, in the demo app to showcase a line
chart with values on the x-axis represented as Doubles not indices/labels.
As well, this can serve as an example on how use ios-charts with date based data.

- Set standard generalized code sign identity values
- Added pipeline to store extra numeric data and send to the renderer
- added xNumericValMin/Max to allow the view to the correct bounds of visible data.
- Extended ChartUtils framework to test for equality between Doubles
@liuxuan30
Copy link
Member

we will get a notification once you watch this repo. You could squash when you reach a small milestone.

@StephanieB5
Copy link
Author

Sounds good to me. Thanks. :)

@RyGuyM
Copy link

RyGuyM commented Mar 22, 2016

@StephanieB5 thanks for the tag! I will have something you to look at in the next couple days. I've added a ChartXAxisRendererNumeric and ChartXAxisRendereTime. I've managed to extend the line and scatter charts to support proper scaling without altering too much of the source.

@StephanieB5
Copy link
Author

@RyGuyM Great! I think extending the charts a few at a time is definitely the way to go. For example, for Bar Charts it makes doesn't make sense to have both axes values represented as Doubles. Whether in horizontal or vertical alignment, Bar charts represent data that has at least one axis with values that are discreet and equidistant.

Btw, I'm thinking regression testing: have you seen the option "Toggle auto scale min max" produce a difference in behaviour? I see in the code where _autoScaleLastLowestVisibleXIndex/_autoScaleLastHighestVisibleXIndex are set but they don't seem to be used anywhere.

I have to admit, without seeing what you've done it's hard to understand how your work fits in with mine. Should we connect over something outside of this thread for more tight knit collaboration?

@StephanieB5
Copy link
Author

Note: To see the "Toggle auto scale min max" scrolling is better. With "auto min max" enabled you can see the slight shift of the graph from the recalculated offset values.

@RyGuyM
Copy link

RyGuyM commented Mar 22, 2016

@StephanieB5 that sounds good, what is the best method for talking outside the thread for you?

@StephanieB5
Copy link
Author

I've updated my profile to show a contact email. Please feel free to reach me through that. Thanks!

@StephanieB5
Copy link
Author

Just an update: @RyGuyM has some work that he's putting the final touches on. I'm going to wait a few days and co-ordinate with @RyGuyM about the best path forward.

@liuxuan30
Copy link
Member

A small heads up: Have you considered/tested combined chart, or other similar charts like bubble chart, candle stick chart (the latter two should be fine naturally)? I don't go in details for this PR right now, but want you to know there is a combined chart might be tricky.
I got a PR #269 for grouped bars support for combined chart, not getting merged yet. If yours comes in first, I will update my PR accordingly. I am not asking that you should support grouped bars right now, but just give some thoughts about current combined chart, if it's fine, then the grouped bar support should not be hard to adopt your changes.

@StephanieB5
Copy link
Author

Thanks for the heads up!

@danielgindi
Copy link
Collaborator

You might want to use Gitter for chatting

@StephanieB5
Copy link
Author

@RyGuyM is putting together his work to create a pull request to resolve this issue. I see no point in creating a competing solution for a problem. :) I'm changing the focus of my work to give space to @RyGuyM. I'll close this pull request but leave my branch as is to allow further development if required.

@StephanieB5 StephanieB5 closed this Apr 5, 2016
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.

5 participants