-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Add new visualization for custom X-Y axes #4185
Closed
Closed
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
2b9fdd5
Add new visualization for custom x-y axis
jeffreythewang 6cbeb70
misc linter fixes
jeffreythewang 8f3711e
Add viz thumbnail for line_xy
jeffreythewang fb67f82
Change show lines to hide lines
jeffreythewang 1311954
Include range filter chart option
jeffreythewang 2eeb502
Validate that state control fields are defined
jeffreythewang 8057798
Remove limiting from xy chart, expose chart to SQL Lab
jeffreythewang 77587ea
Update to new tabbed-styling
jeffreythewang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like the exact same function as in
columns_and_metrics_x
, let's make this DRYThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of being able to plot non grouped-by measures. In some cases I feel that this is a missing option (e.g., box plot).
The non-grouped xy-plot would be even more useful if there were an option to just plot markers without any lines. In other words, a plain scatter plot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I must have mussed that the scatter plot is implemented here, already.
So, I would highly appreciate if the non-grouped code could just stay...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to understand your NOT GROUPED BY use case, is it for optimization purposes in cases where the data is already aggregated? Note that you can still make columns that are used in metrics "groupbable" and group by them. Also note that the cost of grouping something that's already grouped should be fairly cheap.
I think having the two interfaces in one makes things confusing. For the case of the Table viz it was very necessary to allow NOT GROUPED BY, and I'm trying to understand what use case requires that in this context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We, for example, need it to show raw values of our sensor products. So each data point represents one serial number.
You might say that grouping by SN should just do the job. However, grouping by, e.g., the job number, is then more useful.
In other cases there exists real xy measurement data for a product, like e.g. a transfer characteristic, and this again is not useful to be aggregated but has to be investigated for each SN separately.
Limiting the amount of data may of course be challenging then, but this is the user's responsibility in my eyes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mistercrunch The 'NOT GROUPED BY' use case is the same one why it is available on the table visualization. Fundamentally, this general X-Y chart lets you select any column of a table-visualization as the X axis or the Y axis. In short, if you have two numeric columns in a table you should be able to display the data as a line chart regardless of what the 'query' looked like.
The reason for the 'NOT GROUPED BY' section to be included in the table viz is to be able to visualize (list) non-aggregated data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 not grouped by
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about as an alternative to the
GROUPED BY
/NOT GROUPED BY
, if we allowed for a mix of metrics and dimensions in the dropdowns? If no metrics are selected, then we don't aggregate. Perhaps that's the way that the table view should work as well.We're planning a fair amount of work to have a more column-centric approach in the explore view and supporting backwards compatibility on this new chart type may make that harder.