pthom / imgui_bundle

Dear ImGui Bundle: an extensive set of Ready-to-use widgets and libraries, based on ImGui. Start your first app in 5 lines of code, or less. Whether you prefer Python or C++, this pack has your back!
https://pthom.github.io/imgui_bundle/
MIT License
590 stars 62 forks source link

wrapping of implot BeginSubplots appears to have wrong argument type for `row_ratios` and `col_ratios` #207

Closed kuchi closed 2 weeks ago

kuchi commented 2 months ago

I noticed what might be a bug on the wrapping of the BeginSubplots function:

From implot.h:

IMPLOT_API bool BeginSubplots(const char* title_id,
                             int rows,
                             int cols,
                             const ImVec2& size,
                             ImPlotSubplotFlags flags = 0,
                             float* row_ratios        = nullptr,
                             float* col_ratios        = nullptr);

In the python wrapping, it appears to want a float for both the row_ratios and the col_ratios instead of taking a list of floats for each (of length rows and cols). It should also return list of floats as well.

Here is the python type definition:

def begin_subplots(
    title_id: str,
    rows: int,
    cols: int,
    size: ImVec2,
    flags: SubplotFlags = 0,
    row_ratios: Optional[float] = None,
    col_ratios: Optional[float] = None,
) -> Tuple[bool, Optional[float], Optional[float]]:
    pass
pthom commented 2 months ago

Hello Anthony,

As you can see from the number of commits, this one was a bit complex, since I had to write a custom function for the Python bindings and made it so that it is correctly renamed when used from Python.

Anyhow, the signature is now:

def begin_subplots(
    title_id: str,
    rows: int,
    cols: int,
    size: ImVec2,
    flags: SubplotFlags = 0,
    row_ratios: List[float] = List[float](),
    col_ratios: List[float] = List[float](),
) -> bool:
    pass

The CI is ongoing and you will be able to download the wheels from here: https://github.com/pthom/imgui_bundle/actions/runs/8795293678 (They should be ready in 2 hours)

It should also return list of floats as well.

No, this was a new error in the bindings when the values were interpreted as modifiable. It is true that begin_subplots will normalize row_ratios and col_ratios (for example, if you passed row_ratios = [2, 1, 1] the normalized version would be [0.5, 0.25, 0.25]). However, this is not part of the API.

kuchi commented 2 months ago

Pascal,

This almost seems perfect, thank you so much for working on this and sorry it was such a challenge. I built it from source on my apple silicon system. The only remaining bug I see is that the input lists for the row and column ratios are not updated back to python. I believe they should update to return the current ratios if the user interacts with the plots.

Below is a sample script that demonstrates what I am observing.

# Demo of issue that you can't adjust and get back subplot layout ratios
import numpy as np
from imgui_bundle import imgui, immapp, implot
from loguru import logger as log

x = np.log(np.arange(100))
y = np.sin(x)
y2 = y+1
row_ratios = [.6, .4]
col_ratios = [.75, .25]

scater_values = np.array([x,y2]).transpose()
log.warning(f"{scater_values.shape=}")

def gui():
    "Our gui function, which will be invoked by the application loop"

    if implot.begin_subplots("Subplots - try and adjust plot sizes", rows=2, cols=2, size=imgui.ImVec2(-1,-1),
                             row_ratios=row_ratios,
                             col_ratios=col_ratios):
        for i in range(4):
            if implot.begin_plot(f"Play with me {i}"):
                implot.plot_line("line plot", x, y2)

                implot.plot_scatter("scatter plot", x, y)
                implot.end_plot()

        implot.end_subplots()

    # Ratios do not seem to be updated
    print(row_ratios, col_ratios)

immapp.run(gui, with_implot=True)
pthom commented 2 months ago

Hello Anthony,

You're right, I had not suspected that those could be modified. I had to review the API because vectors are not modifiable via the bindings.

https://github.com/pthom/imgui_bundle/commit/b9ce2af00ba2317bdcb44f755d7faac4db09b266#diff-512319a401e7062ec1af5cc784145a9b96f1f0d871406cb908ac97a9008f9052

Thanks for the minimal reproducible demo, it saves a lot of time. Here is its updated version:

import numpy as np
from imgui_bundle import imgui, immapp, implot

x = np.log(np.arange(100))
y = np.sin(x)
y2 = y+1
scater_values = np.array([x,y2]).transpose()

ratios = implot.SubplotsRowColRatios()    # SubplotsRowColRatios is a new type, specialized for this
ratios.row_ratios = [.6, .4]
ratios.col_ratios = [.75, .25]

def gui():
    "Our gui function, which will be invoked by the application loop"

    if implot.begin_subplots("Subplots - try and adjust plot sizes", rows=2, cols=2, size=imgui.ImVec2(-1,-1),
                            row_col_ratios=ratios):
        for i in range(4):
            if implot.begin_plot(f"Play with me {i}"):
                implot.plot_line("line plot", x, y2)

                implot.plot_scatter("scatter plot", x, y)
                implot.end_plot()

        implot.end_subplots()

immapp.run(gui, with_implot=True)
pthom commented 2 months ago

And wheels are being built here: https://github.com/pthom/imgui_bundle/actions/runs/8834742423

You should be able to download them in one hour.