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

Fix potential matplotlib warning when using ui.line_plot.push #4192

Merged
merged 6 commits into from
Jan 14, 2025

Conversation

drkspace
Copy link
Contributor

@drkspace drkspace commented Jan 8, 2025

Description

When trying to use ui.line_plot.push with x or y values that are all equal, you get the following matplotlib warning:

lib/python3.12/site-packages/nicegui/elements/line_plot.py:72: UserWarning: Attempting to set identical low and high ylims makes transformation singular; automatically expanding.
  self.fig.gca().set_ylim(min_y - pad_y, max_y + pad_y)

This PR changes 2 things:

  1. It will not update the x and/or y limits if their respective data points are all the same value.
  2. Add update_y_lims and update_x_lims parameters to push that will disable the limits updating for that push. This will also allow for custom limits to be set (and not reset every call to push).

Example

This example will have a warning on the current version of nicegui.

from nicegui import ui

lp = ui.line_plot(n=1)

i = 0
def update_lp():
    global i
    lp.push([i], [[0]])
    # lp.push([i], [[0]], update_y_lims=False)

    i += 1
ui.timer(1, update_lp)
ui.run()

Not updating the limits if the xs and ys have not difference in value.

Added new parameters to disable updating a specific axis' limits.
@falkoschindler
Copy link
Contributor

Thanks a lot for this pull request, @drkspace!

Checking for x_min != x_max and y_min != y_max is definitely a good idea. These warnings have been there for ages and can be fixed rather easily.

I just wonder if the two additional parameters update_x_lims and update_y_lims are a good choice. It makes sense to provide an opt out for these updates. But what exactly is the use case? If the users want to define custom limits themselves, they can already do so:

line_plot.push([x], [[y]])
with line_plot:
    plt.ylim([0, 1])

Sure, this is not very intuitive, so we might want to provide a parameter like ui.push(..., y_lim=[0, 1]). But in combination with update_y_lims the API gets confusing. Therefore I'm thinking about something like

def push(self, ..., y_limits: Union[None, Literal['auto'], Tuple[float, float]] = 'auto') -> None:

(same for x_limits). Passing None would leave the limits unchanged. The string "auto" would automatically update the limits like before. And a tuple would specify the new limits explicitly. (Note that we tend to avoid abbreviations in NiceGUI, so I'd prefer y_limits over y_lim.)

What do you think? Does that make sense, or am I missing something?

@drkspace
Copy link
Contributor Author

If the users want to define custom limits themselves, they can already do so:

line_plot.push([x], [[y]])
with line_plot:
    plt.ylim([0, 1])

I don't think this is a good solution since it would require knowing the needed limits everywhere push is called. I can certainly imagine a use case where someone has many different places they are calling push. This way would require them to pass around 2/4 different variables to all the places that need them.

What do you think? Does that make sense, or am I missing something?

That makes sense to me. The reason I added the ability to just turn of the auto update is because, in my code, I am setting the limits in 1 part of a class and updating the plot in another. Your suggestion should also allow that.

I've updated the PR.

Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! I just shortened the docstring a little and decided to remove the extra demo because it was too similar to the first one. We can simply use the y_limits parameter there to make the point. I think the docstring is clear enough about its usage.

@falkoschindler falkoschindler added the enhancement New feature or request label Jan 14, 2025
@falkoschindler falkoschindler added this to the 2.10 milestone Jan 14, 2025
@falkoschindler falkoschindler merged commit 95fb61a into zauberzeug:main Jan 14, 2025
1 check passed
@drkspace drkspace deleted the fix-matplotlib-warning branch January 14, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants