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 horizontal and stacked bar chart #23

Merged
merged 3 commits into from
Jun 10, 2019
Merged

Conversation

KarthikRIyer
Copy link
Owner

#22

@KarthikRIyer KarthikRIyer changed the title Implement horizontal and stack bar chart Implement horizontal and stacked bar chart Jun 6, 2019
@KarthikRIyer
Copy link
Owner Author

KarthikRIyer commented Jun 10, 2019

@BradLarson @marcrasi
Can you please review this?

Copy link
Collaborator

@BradLarson BradLarson left a comment

Choose a reason for hiding this comment

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

This all looks good to me.

@KarthikRIyer
Copy link
Owner Author

Thanks for the review. Merging.

@KarthikRIyer KarthikRIyer merged commit e2bc669 into master Jun 10, 2019
@KarthikRIyer
Copy link
Owner Author

KarthikRIyer commented Jun 11, 2019

@BradLarson what if we require just the string variable in case of using words in bar chart? If we use computed properties for string, we'll have to check if while initializing the string or float ones were initialized. To do that we'll have to write a custom getter like this: https://stackoverflow.com/a/49162274/9126612
This doesn't seem very useful...

If we do not want both string and float to be stored simultaneously we can maybe create a separate StringPoint or BarGraphPoint type?

@BradLarson
Copy link
Collaborator

@KarthikRIyer - This is where it might be useful to start thinking about how to provide data for graphs and plots beyond pairs of Float values. If bar charts will sometimes need Strings, sometimes Ints, and the others might want to take in Doubles, Decimals, or other numeric types (I could see taking in Tensor pairs at some point, as a Swift for TensorFlow-specific addition), maybe it's time to contemplate a different input architecture beyond Points.

That's a larger discussion, and might be worth an issue of its own. Would switching to generics be the right path for this? Maybe making plots have their input data comply to specific protocols (representable as pairs of Floats for scatter plots, representable as pairs of String-Float values for bar charts)? Do you still want to explicitly provide pairs, or do you want to switch to having an independent Series for each axis?

@KarthikRIyer
Copy link
Owner Author

What if we make the variables inside point as generics, and accept (String, Numeric generic) in bar chart and (Numeric, Numeric) in Scatter Plot, Line Chart, etc? But in this case when we use the helper functions like getMaxX which uses > operator on x in Point, the generic var in Point would have to conform to Numeric. How can we handle that?

I feel like giving x and y arrays as it is being done now would be simpler for the user
I do not know what Tensor pair is actually, I'll go through the docs.

I've created an issue for this. We can continue the discussion there. #27

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

Successfully merging this pull request may close these issues.

2 participants