mzimmerm / flutter_charts

Charts Library for Flutter, written in Dart with Flutter.
Other
250 stars 42 forks source link

ChartData X values #1

Open jt70 opened 6 years ago

jt70 commented 6 years ago

Hi, it would be useful to have x values in the ChartData for line charts - i.e. instead of ChartData.dataRows being of type List<List<double>>, make it of type List<List<Point>>.

This would be useful for graphing arbitrary functions and time series data. Willing to contribute if you agree, I already have some unpublished code that does this...

mzimmerm commented 6 years ago

That sounds definitely good. I also considered adding ability to set X values as numbers in the past, but then thought for most apps user could manage that by setting X labels as numbers converted to Strings, and went with simplicity with the first version.

If we make a change to allow to set dataRows as List<List>, I'd like us to consider how the (presumably) majority scenario would be handled, that is if user want to simply set X labels, and Y values, and does not care much about the X values; maybe we auto create the List<List> from List<List? And we should consider the opposite, when user set data as List<List> , how would the X labels be set from that, and do we override the ChartData.xLabels, etc.

Also I'd like this library to allow converting external data in various formats to the internal format of ChartData eventually, but that is somewhat beside the point for this change.

Either way X values addition is a good step. I created a branch changes-0.1.3, if you'd consider submitting a pull request against if, I would definitely follow up, although my feedback time may vary depending on work etc.

Another note, I keep README.org the source for README.md, if changes are made to it. I should mention it somewhere.

Thanks, Milan

mzimmerm commented 6 years ago

Also to add some thoughts on providing X values instead of labels that I had (and why I discarded it originally)

If the data x values are

1) Evenly spaced (e.g. 2,4,6,8) then it is easily replaced with labels "2", "4", "6", "8" and everything works the same. 2) Not evenly spaced (e.g. 2, 9, 100, 200). If users need this, they tacitly assume this spacing on the X axis is honoured on the chart. Then things get hard for two reasons: a) is design related: for example, how does a vertical bar chart look like? (Line chart is supposedly the only one where this makes sense? Do we show vertical bar chart evenly spaced?) b) is a technicality of current implementation on the X layout. It assumes evenly spaced steps on the X layout. While I plan a much better layouter, this is in a future, and non-evenly spaced X values would break the layout (or it would "lie" by showing evenly spaced)

So considering 3 b), I feel any ability to allow setting x values e.g. through List<List>, should be done by providing an additional data model, e.g. called ChartData2D, which would be List<List>, and the class would provide an adapter to ChartData (the adapter, should at this time exception uneven spacing of x values, to avoid the issues in 2, before enhancements and design to allow for it).

Any thoughts on this?

Also I meant to ask, could you describe the use case for which you need x values, that would help us going ahead.

Thanks Milan

jt70 commented 6 years ago

Milan - my use case was for a time series line chart (with unevenly spaced dates :) ).

Here is some code I had started, which should be runnable if you want to take a look (sorry it's a bit of a mess) - https://gist.github.com/jt70/41be5e17ede5254bb668233fe535bbd3

Basically the idea is (as you mentioned) to be able to pass in arbitrary data for X/Y values and pass in converters to convert them to double.

Also, it seems like this would only apply to line chart, not bar.

Thanks, Jason

flutter_chart

jt70 commented 6 years ago

As far as ChartData vs. ChartData2D - in my humble opinion, I would allow charts to be created with 1D data for easy use cases, but then convert it internally to ChartData2D. The actual rendering would only deal with 2D data.

mzimmerm commented 6 years ago

Jason, this looks great (I did not run your code yet, but am looking at it now).

A few things:

  1. I agree with you the ChartData2D should be in the internals, and various other ChartData formats (such as the 1D simple use case, ChartDataFromDates , various Json formats etc on the outside, converting to the internal format.

  2. To your use case and chart (code and chart looks great BTW), if I see it correctly, you are painting evenly spaced x labels from the x domain, and the data points that exist in data, is that right? We could do the same in the line chart, while somehow disable the bar chart in those situations, or, maybe, what if we paint the bars (I keep calling it vertical bar, should be column, but anyway) only at points for which x (domain) data exist. So maybe my suggestion to deal with this as invalid for bar chart can be resolved.

  3. I had a few hopefully relatively quick immediate plans for trimming the labels, but after that, if you do plan to use this (flutter charts, a bit heavy name for the contents so far), I can work on incorporating ChartData2D in the internals, and if you were Ok with working on the adaptors, that would be of great help. We would design up the ChartData2D (well the core, dataRows as you said probably makes sense to be List<List< Point> > but I'd like us to also think about optional methods to get the x labels, and defaults based on options:

    • X labels User-Provided or Data-Generated from data, similar to what the Y range labels do now
    • X labels allow / require skip points (to make labels fit if too many)
    • other thoughts, such as data type metadata that would drive the Data-Generated labels format (and eventually tooltips auto generation)
  4. If you could / plan to use this, what is your timeframe?

Thanks for getting in touch and help, hopefully we get a good cooperation going

Milan

jt70 commented 6 years ago

if I see it correctly, you are painting evenly spaced x labels from the x domain, and the data points that exist in data, is that right?

Yes, that is correct.

I had a few hopefully relatively quick immediate plans for trimming the labels

Cool, trimming and maybe font auto-scaling would be pretty sweet.

but I'd like us to also think about optional methods to get the x labels, and defaults based on options

for this I pass in a converter that converts a double back into a string. For example, to round to one decimal point I pass in:

var f = new NumberFormat("###.0"); StringConverter doubleLabelConverter = (v) => f.format(v);

I think this could have some nice defaults based on the data type (or locale for dates).

other thoughts, such as data type metadata that would drive the Data-Generated labels format (and eventually tooltips auto generation)

I pass in an array of label spacings and a preferred number of labels. For example,

var rangeLabelSteps = <double>[1.0, .5, .1, 1.0, 2.0, 5.0, 10.0, 20.0, 50.0, 100.0];

then I passed in 15 for the preferred number of labels. It picks one of the labels spacings that gives the number of labels closest to the preferred based on the data. In my example it came up with 12 labels at a spacing of 2.0 apart. Probably a lot of this could be auto-magic behind the scenes though.

If you could / plan to use this, what is your timeframe?

Maybe a month or so I'm hoping to have my app ready. Right now I just need a basic line chart, but eventually I'll need more advanced stuff. I'm willing to help though, I'll start digging into your code some...

Thanks!

mzimmerm commented 6 years ago

if I see it correctly, you are painting evenly spaced x labels from the x domain, and the data points that exist in data, is that right?

Yes, that is correct.

Thanks

I had a few hopefully relatively quick immediate plans for trimming the labels

Cool, trimming and maybe font auto-scaling would be pretty sweet.

I will work on this.

but I'd like us to also think about optional methods to get the x labels, and defaults based on options

for this I pass in a converter that converts a double back into a string. For example, to round to one decimal point I pass in:

var f = new NumberFormat("###.0"); StringConverter doubleLabelConverter = (v) => f.format(v);

I think this could have some nice defaults based on the data type (or locale for dates).

Sounds good

other thoughts, such as data type metadata that would drive the Data-Generated labels format (and eventually tooltips auto generation)

I pass in an array of label spacings and a preferred number of labels. For example,

var rangeLabelSteps = <double>[1.0, .5, .1, 1.0, 2.0, 5.0, 10.0, 20.0, 50.0, 100.0];

then I passed in 15 for the preferred number of labels. It picks one of the labels spacings that gives the number of labels closest to the preferred based on the data. In my example it came up with 12 labels at a spacing of 2.0 apart. Probably a lot of this could be auto-magic behind the scenes though.

That is great. I do a range scaling in a probably unnecessarily complicated way .. this sounds more efficient :)

If you could / plan to use this, what is your timeframe?

Maybe a month or so I'm hoping to have my app ready. Right now I just need a basic line chart, but eventually I'll need more advanced stuff. I'm willing to help though, I'll start digging into your code some...

That sounds good. That reminds me I also skipped a step before publishing, there is so many todos in a weird precedence order (todo -1, 0, 1,2 oh no) . I want to also take a path cleaning it up and also add docstrings.

To contribute to this project how what would you prefer to do? Pull request from a fork? Or i should be able to create a branch in the repo that you own and you can work on it (i'd have to look how to do that, only did pull requests from a fork before). Please let me know what (or what else) you prefer

Thanks!

jt70 commented 6 years ago

Hi, I should be able to just fork and create a pull request from that. I should have some time this weekend to look at it. Thanks!

jt70 commented 6 years ago

Hi @mzimmerm - could you tell me how you run the example program? I would expect there to be android/ios folders somewhere in the project. Did you forget to add these to git or should I just create a new project that depends on flutter_chart?

Thanks, Jason

mzimmerm commented 6 years ago

Hi Jason:

I just pushed the added android and ios directories to master.. Yes, they were on gitignore, somehow I did not realize they are needed to run. Sorry about that.

I am able to run it both in emulator and also by connecting a device in debug mode.

Could you check and let me know if this worked -

Thanks

PS1: Are you using IntelliJ? PS2: For the pull requests, I think it will be best if we do them against master. I will try to keep it from major changes - but I did a push of relatively large documentation only change now.

jt70 commented 6 years ago

Cool, thanks, it's working now (on android emulator). Ok, I will do any pull-request against master.

Thanks, Jason

mzimmerm commented 6 years ago

Thanks for feedback Jason.