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

Plot: Respect the "Order by" clause when plotting #821

Closed
struck89 opened this Issue Oct 13, 2016 · 10 comments

Comments

Projects
None yet
5 participants
@struck89
Copy link

struck89 commented Oct 13, 2016

Details for the issue

The plot is done in such a way, that the variable selected as "X", is the one that forces the order of the connecting points, So take a look at this nice loop:
good

I can't turn on the lines, because it does not respect the order I asked (order by t):
bad

Useful extra information

The first color it appeared when making this mock example was YELLOW!!

I'm opening this issue because:

  • DB4S is crashing
  • DB4S has a bug
  • DB4S needs a feature
  • DB4S has another problem

I'm using DB4S on:

  • Windows: ( version: ___ )
  • Linux: ( distro: ___ )
  • Mac OS: ( version: ___ )
  • Other: ___

I'm using DB4S version:

  • 3.9.1
  • 3.9.0
  • Other: ___

I have also:

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 13, 2016

Heh Heh Heh, that's interesting plotting. 😄

This request makes sense. Personally I have no idea how to implement it, but hopefully someone else in the team does. 😀

@revolter

This comment has been minimized.

Copy link
Member

revolter commented Oct 13, 2016

I don't even understand why is this happening :/

@struck89

This comment has been minimized.

Copy link
Author

struck89 commented Oct 13, 2016

@revolter It kind of makes sense that somewhere in the code there is a line that says, order the rows by X.
Because if not, given the definition of a relational query, the order doesn't matter, so it could be that the results would appear like this:
wrong
Instead of like this:
right

It's the exact same plot, but the order of the rows is switched.

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented May 5, 2017

I'll just drop this link here as a reminder for myself:
http://www.qcustomplot.com/documentation/classQCPCurve.html

@struck89 struck89 referenced this issue Jan 4, 2018

Closed

Change axis limits #838

4 of 13 tasks complete

@mgrojo mgrojo self-assigned this Jan 6, 2018

mgrojo added a commit that referenced this issue Jan 7, 2018

Plot: Respect the "Order by" clause when plotting
The tables/queries sorted by X are drawn using QCPGraph as before.

Since QCPGraph does automatically sort by X, we change to QCPCurve that
requires a third data vector to reflect the order. We get that from the
current row order.

In the case of curves, only None and Line is supported as line style.

Since the order is now important for the plot, it is automatically updated
whenever the user sorts by another column in the browsed table.

This addresses issue #821 and indirectly fixes the problem of incorrect
point->row selection link when the table is not sorted by X, reported in
issue #838.
@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented Jan 7, 2018

@struck89 I've implemented a change for using the QCPCurve when the table/query is not sorted by the X field. That looses some options for the line style, but only for that case. When the table is already sorted by X, everything stays the same. Could you test with the next nightly build how does it work with your data?

Indirectly, this solves too the problem that you reported in #838 about the incorrect row being selected.

@struck89

This comment has been minimized.

Copy link
Author

struck89 commented Jan 8, 2018

@mgrojo
The result is awesome. Thank you a lot
There are now some bits here and there which need some tweaking. The fetch-all button is not working properly now:
https://we.tl/YpPuaVtxwR

So, now that we have reached this amazing point, now I will throw you another challenge to think about:
Two or more parametric curves:
image

You would need for curve 1, x1 and y1, for curve 2, x2 and y2.
🕺

(btw, in my case, I don't see any loss in plotting functionality)

mgrojo added a commit that referenced this issue Jan 8, 2018

Use the correct function for checking the count of plotted items
Now that the plot may contain graphs and curves, the plottableCount()
function must be used instead of graphCount() for checking whether the plot
has any plotted items.

This had the effect reported in issue #821 of breaking the enable check for
the fetch-all button.
@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented Jan 8, 2018

@struck89 Good catch about the fetch-all button. This is due to the oddness that the QCustomPlot API has for the curves. It is clearly an afterthought. That should be fixed now.

Your new idea deserves some thought 🤔 Could you open a new issue for that and develop a bit the user story 😄 It will probably not materialise soon, since we want now to get ready for the 3.11.0 release.

@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented Jan 8, 2018

By the way, the bit lost for the plot when there are curves is that only None and Line can be selected as line type. The other styles are only allowed by QCustomPlot for graphs.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jan 8, 2018

Sounds like the capabilities of the Plot pane are really expanding. Should we create a "How to use the Plot pane" page on the wiki (with screenshots)?

@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented May 1, 2018

Wiki suggestion copied in #226. It seems this can be closed now.

@mgrojo mgrojo closed this May 1, 2018

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