highfestiva / finplot

Performant and effortless finance plotting for Python
MIT License
922 stars 187 forks source link

[BUG] Multiple charts on GUI, crosshair only working on last one #393

Closed Yazzito closed 10 months ago

Yazzito commented 1 year ago

Requirements (place an x in each of the [ ])**

Code to reproduce

import sys
from PyQt5.QtWidgets import *
import finplot as fplt
import yfinance

class MainWindow(QMainWindow):
    def __init__(self):
        super().__init__()
        self.setGeometry(0, 0, 800, 400)
        layout = QVBoxLayout()

        # Create 3 charts to illustrate the problem
        self.chart1 = MyChartPlot(self)
        self.chart2 = MyChartPlot(self)
        self.chart3 = MyChartPlot(self)
        layout.addWidget(self.chart1)
        layout.addWidget(self.chart2)
        layout.addWidget(self.chart3)
        # Create central widget
        self.main_widget = QWidget()
        self.main_widget.setLayout(layout)
        self.setCentralWidget(self.main_widget)
        # Show fplot & QT
        self.window().axs = [self.chart1.fplt_widget, self.chart2.fplt_widget, self.chart3.fplt_widget]
        fplt.show(qt_exec=False)
        self.show()

class MyChartPlot(QWidget):
    def __init__(self, parent):
        super(MyChartPlot, self).__init__(parent)
        # Download data
        self.df = yfinance.download('AAPL')
        # Create layout and plot
        layout = QVBoxLayout(self)
        self.fplt_widget = fplt.create_plot_widget(self.window())
        fplt.candlestick_ochl(self.df[['Open', 'Close', 'High', 'Low']])
        layout.addWidget(self.fplt_widget.ax_widget)
        self.setLayout(layout)

if __name__ == '__main__':
    app = QApplication(sys.argv)
    ex = MainWindow()
    sys.exit(app.exec())

Describe the bug

Hello Highfestiva, I hope you are well.

The bug I am running into is when creating multiple chart widgets on a GUI, the crosshairs only work on the last chart. (I do have an updated _axs = [list of chart widgets] and I am using qt_exec during the fplt.show)

ie. If you move the mouse cursor over the top 2 charts, nothing happens. If you move it over the bottom one, you will see the crosshair move on all 3 charts simultaneously.

You can run the example code to see the issue. The charts themselves display and work fine. The problem is that the crosshair only moves on the last chart. Moving the mouse cursor over any chart other than the last one does nothing.

PS. I modified the example code from someone's question from stackoverflow & used your response to him to create this testcase. In the end though, the crosshairs didn't work for him either. So I wanted to add as a bug and get your take on it? https://stackoverflow.com/questions/64074217/finplot-as-a-widget-in-layout

Expected behavior

Each chart should have it's own working FinCrossHair and it should work when the mouse is moved over each chart.

Screenshots

I don't believe screenshots are needed here, example is quite clear, but please let me know if you need to see anything.

Reproducible in:

OS: Windows 10 finplot version: 1.9.0 pyqtgraph version: pyqtgraph-0.13.1 pyqt version: PyQt5-5.15.7

--- Please let me know if I can provide any other information or if you have any ideas I can try. Happy to debug and try things out if there is a real issue.

highfestiva commented 1 year ago

Follow the instructions in examples/dockable.py, i.e. create three "rows" in the create_plot_widget() call instead of making three individual ones. GL!

Yazzito commented 1 year ago

Hello HighFestiva,

Thanks for your feedback, but your solution does not work for my implementation. In fact, it's not a proper solution at all for most generic cases. You already have a working and functional chart, why limit the crosshairs by master? Let each chart have it's own crosshair.

So I decided to debug the issue and now have a fix in place that works properly.

I'll describe below what the problem is and how I fixed it.

PROBLEMATIC CODE: master_data[master] = dict(proxymm=proxy, last_mouse_evs=None, last_mouse_y=0)

In the example in the OP, the 'master' for all 3 create_plot_widgets is self.window(). Thus, every create_plot_widget overwrites the master_data for the previous ones. This is why only the last chart created has a working FinCrossHair.

Simple Fix: create an additional level of dictionary. master_data[master][ax] = dict(proxymm=proxy, last_mouse_evs=None, last_mouse_y=0)

Now, master_data stores not only the master for each widget but also the axis (ax). Since the AX is unique, all plots now have their own working FinCrossHair.

1) _mouse_moved function was modified to include an extra parameter, ax. So we could store it in master_data as mentioned above.

2) All calls to _mouse_moved now need to be passed AX as well as master.

I don't know if you plan to incorporate this fix into finplot (I would) but if you need anything (info, code, testing, etc.) please let me know.

From my side, this issue is now resolved/closed. If you'd like to close the ticket, please go ahead.

Thanks again for your help and best wishes!

highfestiva commented 1 year ago

Thanks, a great solution! I'll incorporate it in the code. Next time, feel free to create a pull request so I can simply review and merge it into the master branch. You'll get the fix you need asap, and my work is reduced, win-win!

highfestiva commented 1 year ago

I added it on the viewbox, as that was a better fit for whence the event is sent most of the time. Fix in 10e63e9b.