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

Bar charts and Axis Type column in Plot Dock #1302

Merged
merged 4 commits into from Jan 26, 2018

Conversation

Projects
None yet
5 participants
@mgrojo
Copy link
Contributor

mgrojo commented Jan 20, 2018

This PR adds support for bar charts in the Plot Dock. A string column is used as bar labels. This is specially thought for GROUP BY queries.

A new column for displaying the axis type is added, so the plot results are predictable for the user. This column stores now the internal type enumeration.

Only columns that are correctly plotted as Y axis are selectable for that.

There are caveats, like plotting more than one Y axis, since the bars overlap. To alleviate this the additional bars are translucent, but this kind of graph is maybe not very interesting. Should the user have the option to stack the bars when that makes sense?

I'm confident of the changes, but since we plan to make soon a release, I prefer to commit this as a pull request, so it can be reviewed. I'd like opinions also about the new Axis Type column, since it is not actually needed for the implementation (maybe there should be two PR, but this is already done).

imagen

mgrojo added some commits Jan 20, 2018

Bar charts and Axis Type column in Plot Dock
String type columns are now selectable as X axis. In that case a bar chart
is plotted where the column values are used as bar labels.

A new Axis Type column is added to the axis selection table, so the user
knows before-hand how the plot will be drawn.

Line type and point shape combo-boxes are not enabled for bar charts, since
they don't make sense.

Columns that make no sense for the Y axis as strings and date/times are not
selectable for that axis.
Simplify the axis type storing, translucent additional bars and What'…
…s This

Now that we have a proper type column in the plot selection widget, the
internal type storing can be simplified.

If the plot has more than one Y selections for one X of label type (bar
charts) the first bar is opaque, but the additional are translucent in
order to be seen, since they will overlap.

What's This information added to explain all the possible selections for
plotting.
@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jan 20, 2018

Should the user have the option to stack the bars when that makes sense?

Definitely.

Co-incidentally, I've been looking at doing some chart building stuff recently too (mainly box plots though), as a way to get myself back into the dbhub.io coding. Not using Qt for it, as the Go libraries for Qt just aren't complete enough yet for my purposes.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jan 20, 2018

@SilvioGrosso

This comment has been minimized.

Copy link

SilvioGrosso commented Jan 21, 2018

Hello everyone,

Just tested this new feature on Windows 10 (64 bit): after having installed the 64.exe version.
Everything looks fine !
Thansk a lot indeed 👍

db-manager_group-by_plot

A little glitch also present in the previous versions for the plot feature is that the top window, the one with all the settings for the colums (X, Y, Axis type), is usually hidden when you select the Plot the first time. To make it appear you must drag down the other window with your plot.
IMHO, this is a bit confusing for new users.
In short, when you select the plot both windows should be visible at first.
At present, only the window for the plot itself is visible and I have to drag down its upper border in order to visualize the second window with all the options (X, Y, Axis type).

If needed I can open a new issue since:
I have encountered a bug concerning the new autocompletion feature for columns.
In short, with group by statements the columns are not recognized and you are forced to write by hand their names (or you can drag them by getting them into their schema window).
In short when you write:
count-sum-etc ( the column is not autocompleted anymore into the brackets)
At first I have thought this bug was due to the brackets () but in reality the problem occurs only when you work with group by statements.

E.g:
Select sum ( autocompletion for names of columns does not work anymore... )

EDIT
This bug only occurs when you do not write the name of the table (from table1) in your SQL code.
If you write:
Select
from table1
group by column
and then write sum ( ) the autocompletion works !

BTW, since we are in the Pull requests topics, it would be extremely cool to have also the "math extension" [1] available into the next upcoming release :-)
I am available to test it on Windows if needed.

[1] #1224

Stacked or grouped bars and plot legend
Two new options added to the context menu of the plot:

- Stacked bars: switches between stacked bars or grouped bars. The former
overlapped layout is avoided since it doesn't make much sense.

- Show legend: toggles the display of a plot legend with a translucent
background. Possible future improvement is dragging the legend with the
mouse.
@mgrojo

This comment has been minimized.

Copy link
Contributor Author

mgrojo commented Jan 21, 2018

@justinclift Thanks for the builds. I bring now more "material".

I've committed the option for the stacked bars. When they are not stacked they are grouped together, that makes much more sense as overlapped (but that was surprisingly the default layout for QCustomPlot). The option is in the context menu of the plot. Maybe it should replace the Line type and Point shape combo-boxes for bar charts but it was easier this way. Maybe some day.

I've also added an option for displaying a plot legend for any kind of plot. Maybe some day it would be draggable with the mouse 😄

"Stacked bars" checked and "Show legend" checked:
imagen

"Stacked bars" unchecked (grouped) and "Show legend" unchecked (defaults):
imagen

@SilvioGrosso I'm glad that my enhancement to DB4S can contribute a bit to the advance of Science 😃

Those problems that you mention may deserve each one a new issue. The one about the plot isn't happening in my environment. Maybe a screenshot can help discover the cause. The one about completion happens for all the functions that display a "calltip". The workaround is to click on that calltip to remove it and then the completion works again. This is how QScintilla works, I don't know if we could work around it.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jan 21, 2018

Wow @mgrojo, you're getting SO much stuff done. Our next release is really going to be well named. 😄

New builds for Windows with this new commit are ready:

BTW, since we are in the Pull requests topics, it would be extremely cool to have also the "math extension" [1] available into the next upcoming release :-)

@SilvioGrosso Good point. I really need to get around to making that happen. 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jan 21, 2018

Sent a tweet about this to our Twitter account too. Might be useful for getting feedback. 😄

    https://twitter.com/sqlitebrowser/status/955160333924085761

@mgrojo

This comment has been minimized.

Copy link
Contributor Author

mgrojo commented Jan 21, 2018

Wow @mgrojo, you're getting SO much stuff done. Our next release is really going to be well named. 😄

It's currently my pastime 😄

@SilvioGrosso

This comment has been minimized.

Copy link

SilvioGrosso commented Jan 21, 2018

Hello everyone,

Just tested today's new build.
Everything is fine (on Windows 10 with the new 64.exe installer).
THANKS a lot indeed for your hard work: next version is going to be awesome 👍

plot2

@mgrojo

The one about the plot isn't happening in my environment

Yep. It only occours once in a while when you install a new version on Windows.
For instance, today's latest version is fine since both windows for the Plot are visible at once: no need to drag :-)

@justinclift

I really need to get around to making that happen.

Yep. It would be really useful for Windows users 👍

@MKleusberg
Copy link
Member

MKleusberg left a comment

Other than these two minor issues I couldn't find any problem with this either 😄 Nice work 😄


switch (columntype) {
case QVariant::DateTime:
columnitem->setText(PlotColumnType, "Date/Time");

This comment has been minimized.

@MKleusberg

MKleusberg Jan 26, 2018

Member

These should have a tr() around them as well.

This comment has been minimized.

@mgrojo

mgrojo Jan 26, 2018

Author Contributor

Fixed.

break;
case QVariant::String:
columnitem->setText(PlotColumnType, "Label");
break;

This comment has been minimized.

@MKleusberg

MKleusberg Jan 26, 2018

Member

Can you add a default branch? Without one it's producing tons of warnings for me 😉

This comment has been minimized.

@mgrojo

mgrojo Jan 26, 2018

Author Contributor

Fixed. I don't get any warnings, either compiler version difference or difference in flags.

Translatable strings and default case branch
This addresses review comments by @MKleusberg in PR #1302.

@mgrojo mgrojo merged commit 87e1c27 into master Jan 26, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@mgrojo mgrojo deleted the bar_charts branch Jan 26, 2018

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jan 26, 2018

Fantastic! 😄

@chrisjlocke Feel like creating some animated gif things showing how to use the new bar charts? They'd make for awesome instructions on the wiki. 😄

@struck89

This comment has been minimized.

Copy link

struck89 commented Jan 27, 2018

My apologies for coming too late to the party.
The feature looks great. Unfortunately I don't have a use case right now, but sure I will in the coming future. Thanks!

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