Skip to content

Better error message when passing global callbacks to Gui and Gui.run#2837

Open
Krishnachaitanyakc wants to merge 1 commit into
Avaiga:developfrom
Krishnachaitanyakc:fix/2764-better-error-global-callbacks
Open

Better error message when passing global callbacks to Gui and Gui.run#2837
Krishnachaitanyakc wants to merge 1 commit into
Avaiga:developfrom
Krishnachaitanyakc:fix/2764-better-error-global-callbacks

Conversation

@Krishnachaitanyakc

Copy link
Copy Markdown

Summary

Fixes #2764

  • Gui.__init__(): Now raises a clear TypeError with actionable guidance when users mistakenly pass global callback names (on_change, on_init, on_navigate, on_action, on_exception, on_status, on_page_load, on_user_content) as keyword arguments. The error message directs users to assign them as attributes on the Gui instance instead (e.g., gui.on_change = handle_change).
  • Gui.run(): Now emits a warning when global callback names are passed as keyword arguments, since they were previously silently ignored due to not being recognized config keys. The warning provides the same actionable guidance.
  • Unknown keyword arguments to Gui.__init__() also produce a clear TypeError instead of being silently accepted.

Before

# Raises: TypeError: Gui.__init__() got an unexpected keyword argument 'on_change'
Gui(pages=pages, on_change=handle_change).run()

# Silently does nothing - very confusing to troubleshoot
Gui(pages=pages).run(on_change=handle_change)

After

# Raises: TypeError: Gui.__init__() got unexpected keyword argument(s): on_change.
# Global callbacks cannot be passed to the Gui constructor.
# Please assign them after creating the Gui instance, for example: gui.on_change = on_change
Gui(pages=pages, on_change=handle_change).run()

# Warns: Gui.run() received global callback argument(s): on_change.
# These arguments are not supported by Gui.run() and will be ignored.
# Please assign them to the Gui instance directly, for example: gui.on_change = on_change
Gui(pages=pages).run(on_change=handle_change)

Test plan

  • Added test_init_rejects_global_callback_with_helpful_message - verifies single callback in __init__
  • Added test_init_rejects_multiple_global_callbacks_with_helpful_message - verifies multiple callbacks in __init__
  • Added test_init_rejects_unknown_kwargs - verifies unknown kwargs still raise TypeError
  • Added test_run_warns_on_global_callback_kwargs - verifies warning in run()
  • Existing tests continue to pass (normal Gui() construction is unaffected)

Fixes Avaiga#2764

- Gui.__init__() now raises a clear TypeError with guidance when global
  callback names (on_change, on_init, on_navigate, etc.) are passed as
  keyword arguments, directing users to assign them as attributes instead.
- Gui.run() now emits a warning when global callback names are passed,
  since they were previously silently ignored.
- Added unit tests for both behaviors.
@FabienLelaquais

Copy link
Copy Markdown
Member

Thanks a lot @Krishnachaitanyakc for your contribution.
You code is fine and could be merged as is.

However I'm reluctant to let this go through. The reason is this 'problem' is very, very generic. Implementing this guard rail in these two methods (Gui.__init__ and Gui.run) would break the homogeneity of the whole package APIs: check, in every function, for potentially additional parameters what might be due to confusion from the programmer's part.

Please let me sort this out with @arcanaxion, who originally created the issue you're fixing.
I feel like it is worth checking, as often as possible, the parameter names when a kwargs is in place (like in Gui.run()), but not when it's not the case (like in Gui.__init__()). Because the Python linter cannot spot invalid parameters, that would be useful.
Functions that do not use kwargs should however be spotted by linters and I would recommend to leave these functions alone.

What do you think?
And @arcanaxion would you agree with my feeling?

Thanks again

@arcanaxion

Copy link
Copy Markdown
Contributor

Some context to the issue is that:

  1. I've seen many users try to pass on_change (etc.) to Gui.__init__
  2. Copilot wants to do that too
Screenshot 2026-05-04 080938

Given that they reach out for help, they weren't able to easily figure out the proper way.

The reason is this 'problem' is very, very generic. Implementing this guard rail in these two methods (Gui.init and Gui.run) would break the homogeneity of the whole package APIs: check, in every function, for potentially additional parameters what might be due to confusion from the programmer's part.

Correct me if I'm wrong, but I interpret your point about implementing a guard rail here as sort of being a slippery slope for why not implement guard rails in other places.

IMO, this particular quirk is quite exceptional because of Taipy's behaviour of implicitly recognizing and using a function named on_change (etc.). This is convenient, but to me also a footgun that deserves special handling.

I'm not sure that we actually ever properly document this implicit feature (and also how it works when there are global callbacks in submodules. E.g. __main__.on_change and home.on_change, on_init, etc.).

I think we should start with better documenting this behaviour first, and then reevaluate if this PR is necessary. I agree that adding kwargs to Gui.init may be overkill.

@FabienLelaquais

Copy link
Copy Markdown
Member

Thanks you for your input, @arcanaxion.

Yes, your point is completely correct: why check here and not elsewhere?
The one sentence: "I've seen many users try to pass on_change (etc.) to Gui.init" might be enough to convince me.
It does make sense in this specific situation because well, many people are confused (including LLMs) and this would fix that.
We might want to even explicitly add the on_* parameters to __init__() to fix this once and for all. Then it would not become an outlawyer.

What do you think?

What about you @Krishnachaitanyakc?

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

Successfully merging this pull request may close these issues.

[Refactor] Better error message when trying to pass global callbacks to Gui and Gui.run

3 participants