Repository navigation
Make components always truthy - #4078
monsterlag wants to merge 1 commit into
Conversation
Component defined __len__ but not __bool__, so truthiness fell back to the number of children and a component without children (or without a children prop, like DataTable) was falsy. Fixes plotly#2906.
|
|
Thanks for the PR! We'll take a closer look and follow up. |
T4rk1n
left a comment
T4rk1n
left a comment
There was a problem hiding this comment.
This breaks pages apps that use an empty placeholder layout, so I'd block on it as is.
dash.py:1894 does if not self.layout and self.use_pages: self.layout = page_container. Today app.layout = html.Div() with use_pages=True gets swapped for page_container. With __bool__ always True, the empty Div is kept. Then every request fails with "dash.page_container not found in the layout". The app still starts, but / and every other page return 500. I reproduced it with dash_duo: the page loads on dev and never loads with this change.
Can you keep the old behavior at that check? For example self.layout is None or (isinstance(self.layout, Component) and not len(self.layout)). And can you add an integration test for the pages case?
This also changes public behavior: user code that does if component: to mean "has children" silently flips. Please add a CHANGELOG entry that says so.
| for t in self._traverse_ids(): | ||
| yield t.id # type: ignore[reportAttributeAccessIssue] | ||
|
|
||
| def __bool__(self): |
There was a problem hiding this comment.
This also changes the result of if not self.layout in dash.py:1894. An empty layout with use_pages=True is no longer replaced with page_container, and every request then errors. Can you update that check so an empty component still counts as unset there?
| dcc.Input(children="invalid children") | ||
|
|
||
|
|
||
| def test_debc031_bool_is_always_true(): |
There was a problem hiding this comment.
This covers bool() itself but not the case that breaks. Can you add a dash_duo test under tests/integration/multi_page/: use_pages=True, app.layout = html.Div(), check that the home page renders? It fails with the current change, so it guards the fix.
Note
AI-generated, not reviewed by a human. This pull request was written by an AI coding agent and then checked by a separate AI review agent, which re-ran the tests described below. No human has reviewed the code. It is submitted by @monsterlag, who runs these agents. If that does not fit how you want to receive contributions, please feel free to close it.
Componentdefines__len__(the number of items in its children tree) but not__bool__, so Python falls back to__len__for truthiness. Any component without children is falsy:bool(html.Div())isFalse, andbool(dash_table.DataTable(data=[...]))is alwaysFalsebecause DataTable has nochildrenprop. That breaks the usualcomponent or fallbackandif component:patterns.This adds
Component.__bool__that always returnsTrue, as suggested in the issue.len()and iteration are unchanged.One behavior change to note: in
_setup_server,if not self.layout and self.use_pages:no longer replaces an explicitly assigned empty layout (for exampleapp.layout = html.Div()) withpage_container. A layout that was never set (None) is still replaced as before.Fixes #2906
Contributor Checklist
Component.__bool__returningTrueoptionals
CHANGELOG.mdHow tested
Python 3.11 on Linux,
pip install -e .[ci,dev,testing,celery,diskcache]. I used the builtdcc,html,dash_table,depsand renderer build files from the 4.5.0rc1 wheel instead of runningnpm run build.test_debc031_bool_is_always_trueintests/unit/development/test_base_component.py:pytest tests/unit/development/test_base_component.py -k debc031fails before the change (assert bool(Component())) and passes after it.pytest tests/unit/development/test_base_component.py: 31 passed.pytest tests/unit: 723 passed, 5 failed, 2 errors with the change. Without it: 722 passed, with the same 5 failures and 2 errors. The failures are environment-only:test_browser_smoke(no browser available) andtest_main_module_alias.dash_table.DataTable(...) or "no table". Before the change it returned"no table". After it, it returned the DataTable.black --checkandflake8on both changed files passed, andpylintonbase_component.pygave 10.00/10.I did not run a
dash_duobrowser test because no Chrome is available in my environment. I left out a CHANGELOG entry because it needs the PR number. I can add one under Fixed.The same truthiness change also affects the
validation_layoutchecks (dash.pyaround lines 1030 and 1236): avalidation_layoutset to a childless component is now treated as set rather than overwritten. Withuse_pages, an explicitly empty layout now reaches Dash's existing "dash.page_containernot found in the layout" error instead of being silently replaced. The 2 errors in the unit run (test_app_runners.pysmoke tests) are environmental and identical on the base branch.