Skip to content
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

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

Open
kuchi opened this issue Apr 22, 2024 · 4 comments

Comments

@kuchi
Copy link

kuchi commented Apr 22, 2024

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
Copy link
Owner

pthom commented Apr 23, 2024

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
Copy link
Author

kuchi commented Apr 25, 2024

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
Copy link
Owner

pthom commented Apr 25, 2024

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.

b9ce2af#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
Copy link
Owner

pthom commented Apr 25, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants