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

[🐛 BUG] Multi-value slider with lov binds value to the post-adapter lov elements #2290

Closed
1 of 7 tasks
arcanaxion opened this issue Nov 29, 2024 · 7 comments · Fixed by #2301
Closed
1 of 7 tasks
Assignees
Labels
🖰 GUI Related to GUI 🟥 Priority: Critical Must be addressed as soon as possible 🔒 Staff only Can only be assigned to the Taipy R&D team

Comments

@arcanaxion
Copy link

arcanaxion commented Nov 29, 2024

What went wrong? 🤔

Example code:

from taipy.gui import Gui
import taipy.gui.builder as tgb

aggregation_list = ["minute", "hour", "day", "week"]
selected_aggregation = [aggregation_list[0], aggregation_list[1]]

def aggregation_adapter(aggregation):
    return aggregation.title()

with tgb.Page() as page:
    tgb.text("# Adapter modifying bound value for multi-value slider", mode="md")
    tgb.slider("{selected_aggregation}", lov="{aggregation_list}", label="Aggregation", adapter=aggregation_adapter)
    tgb.text("{selected_aggregation}")

def on_change(state, var_name, var_value):
    print(var_name, type(var_value), var_value)

if __name__ == "__main__":
    gui = Gui(page=page)
    gui.run()

In develop, modifying the slider displays:

image

Seems to be a regression -- was working as expected in 4.0.1.

Expected Behavior

Values should be lowercase

Browsers

Chrome, Firefox

OS

Windows, Linux

Version of Taipy

develop

Acceptance Criteria

  • A unit test reproducing the bug is added.
  • Any new code is covered by a unit tested.
  • Check code coverage is at least 90%.
  • The bug reporter validated the fix.
  • Related issue(s) in taipy-doc are created for documentation and Release Notes are updated.

Code of Conduct

  • I have checked the existing issues.
  • I am willing to work on this issue (optional)
@arcanaxion arcanaxion added the 💥Malfunction Addresses an identified problem. label Nov 29, 2024
@FlorianJacta FlorianJacta added 🖰 GUI Related to GUI 🟧 Priority: High Must be addressed as soon 🟥 Priority: Critical Must be addressed as soon as possible and removed 🟧 Priority: High Must be addressed as soon labels Nov 29, 2024
@FredLL-Avaiga FredLL-Avaiga self-assigned this Nov 29, 2024
@FredLL-Avaiga
Copy link
Member

Does it work for single value selection ?

@FredLL-Avaiga
Copy link
Member

FredLL-Avaiga commented Nov 29, 2024

If you want an id different from your label, you need to specify it in your adapter:

def aggregation_adapter(aggregation):
    return (aggregation, aggregation.title())

Does that work for you @arcanaxion ?

import taipy.gui.builder as tgb
from taipy.gui import Gui

aggregation_list = ["minute", "hour", "day", "week"]
selected_aggregation = [aggregation_list[0], aggregation_list[1]]

def aggregation_adapter(aggregation):
    return (aggregation, aggregation.title())

with tgb.Page() as page:
    tgb.text("# Adapter modifying bound value for multi-value slider", mode="md")
    tgb.slider("{selected_aggregation}", lov="{aggregation_list}", label="Aggregation", adapter=aggregation_adapter)
    tgb.text("{selected_aggregation}")

def on_change(state, var_name, var_value):
    print(var_name, type(var_value), var_value)

if __name__ == "__main__":
    gui = Gui(page=page)
    gui.run(title="2290 [🐛 BUG] Multi-value slider with lov binds value to the post-adapter lov elements")

@arcanaxion
Copy link
Author

@FredLL-Avaiga Yes, your correction works for me and it makes sense. Indeed the docs state that adapter should return a tuple.

But the inconsistency using my (wrong) method only seems to be affecting multi-value sliders:

from taipy.gui import Gui
import taipy.gui.builder as tgb

aggregation_list = ["minute", "hour", "day", "week"]
selected_aggregation_multiple = [aggregation_list[0], aggregation_list[1]]
selected_aggregation = aggregation_list[0]
selected_aggregation_selector = aggregation_list[0]

def aggregation_adapter(aggregation):
    return aggregation.title()

with tgb.Page() as page:
    tgb.text("# Adapter modifying bound value for multi-value slider", mode="md")
    tgb.slider("{selected_aggregation_multiple}", lov="{aggregation_list}", adapter=aggregation_adapter)
    tgb.text("{selected_aggregation_multiple}")

    tgb.slider("{selected_aggregation}", lov="{aggregation_list}", adapter=aggregation_adapter)
    tgb.text("{selected_aggregation}")

    tgb.selector("{selected_aggregation_selector}", lov="{aggregation_list}", adapter=aggregation_adapter)
    tgb.text("{selected_aggregation_selector}")

def on_change(state, var_name, var_value):
    print(var_name, type(var_value), var_value)

if __name__ == "__main__":
    gui = Gui(page=page)
    gui.run()

image

Probably this should either work consistently, or perhaps raise an error if the adapter is not returning a 2-element tuple.

@jrobinAV jrobinAV added the 🔒 Staff only Can only be assigned to the Taipy R&D team label Dec 2, 2024
@FredLL-Avaiga
Copy link
Member

@FabienLelaquais Doc ?

@FredLL-Avaiga FredLL-Avaiga added 📄 Documentation Internal or public documentation 🟩 Priority: Low Low priority and doesn't need to be rushed and removed 🟥 Priority: Critical Must be addressed as soon as possible 💥Malfunction Addresses an identified problem. labels Dec 3, 2024
@arcanaxion
Copy link
Author

@FredLL-Avaiga I think the issue I'm having is unrelated to adapters.

I'm comparing 3 versions:

  1. 3.0.1 (I'm updating an old application using this version)
  2. 4.0.1 (latest stable release)
  3. develop (because I want the fix in add current scope variables when evaluating an expression #2250)

Problem with tree

First I noticed that the "tree" element was returning ids instead of lov elements. Example copied from the docs:

from taipy.gui import Gui, State, Markdown
from typing import Any

users = [
    {"id": "231", "label": "Johanna", "year": 1987, "children": [{"id": "231.1", "label": "Johanna's son", "year": 2006}]},
    {"id": "125", "label": "John",    "year": 1979, "children": []},
    {"id": "4",   "label": "Peter",   "year": 1968, "children": []},
    {"id": "31",  "label": "Mary",    "year": 1974, "children": []}
    ]

user_sel = users[2]

page = Markdown(
"""
# Trees

<|{user_sel}|tree|lov={users}|>

""")

def on_change(state: State, var_name: str, var_value: Any):
    print(var_name, type(var_value), var_value)

if __name__ == "__main__":
    gui = Gui(page=page)
    gui.run(run_browser=False)

When I click on John:
- 3.0.1 and 4.0.1 would have var_value: [{'id': '125', 'label': 'John', 'year': 1979, 'children': []}]
- develop would have var_value: ['125']

Setting not value_by_id did not change anything, and that is already the default according to the docs.

This made me look at the example in this github issue again.

Slider, without adapter

See the following. I'm not using tgb because the tgb.selector didn't seem to work for me in 3.0.1:

from taipy.gui import Gui
import taipy.gui.builder as tgb


aggregation_list = [("minute", "Minute"), ("hour", "Hour"), ("day", "Day"), ("week", "Week")]
selected_aggregation_slider_multiple = [aggregation_list[0], aggregation_list[1]]
selected_aggregation_slider_single = None
selected_aggregation_selector_single = aggregation_list[0]

page = Markdown(
"""
# No adapter

<|{selected_aggregation_slider_multiple}|slider|lov={aggregation_list}|>

<|{repr(selected_aggregation_slider_multiple)}|>

<|{selected_aggregation_slider_single}|slider|lov={aggregation_list}|>

<|{repr(selected_aggregation_slider_single)}|>

<|{selected_aggregation_selector_single}|selector|lov={aggregation_list}|>

<|{repr(selected_aggregation_selector_single)}|>

""")

def on_change(state, var_name, var_value):
    print(var_name, type(var_value), var_value)


if __name__ == "__main__":
    gui = Gui(page=page)
    gui.run(run_browser=False, use_reloader=True)

3.0.1

This has the most expected behaviour to me:

3.0.1.slider.mp4

All 3 visual elements are giving the selected lov element (a tuple) as its value.

4.0.1

Here, a possibly unrelated issue, the single value slider becomes strange:

4.0.1.slider.mp4

I'm expecting a single value slider but it's becoming a multi-value slider.

develop

Here we start seeing what I think this github issue is related to:

develop.slider.mp4

The multi-value has renders the expected initial state. After interaction, the multi-value slider starts giving ids instead of lov elements, even though value_by_id is not set.

The unexpected behaviour for single value slider is also still present.

For all 3 versions, the selector has the same expected behaviour.

If this is actually just yet-to-be-documented behaviour for a future release, please let me know and I will go back to stable 😅

@FredLL-Avaiga
Copy link
Member

I'll look into it
thanks for the detailed report @arcanaxion

@FredLL-Avaiga FredLL-Avaiga added 🟥 Priority: Critical Must be addressed as soon as possible and removed 🟩 Priority: Low Low priority and doesn't need to be rushed 📄 Documentation Internal or public documentation labels Dec 3, 2024
@FredLL-Avaiga
Copy link
Member

There was indeed a bug in the way we handle multiple selection in lov based components

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🖰 GUI Related to GUI 🟥 Priority: Critical Must be addressed as soon as possible 🔒 Staff only Can only be assigned to the Taipy R&D team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants