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-Docs state element adapters should return tuple[str, str | Icon] #1248

Open
arcanaxion opened this issue Jan 3, 2025 · 1 comment
Open
Labels
📄 Documentation Internal or public documentation 🖰 GUI Related to GUI 🆘 Help wanted Open to participation from the community 💥Malfunction Addresses an identified problem. 🟨 Priority: Medium Not blocking but should be fixed soon

Comments

@arcanaxion
Copy link
Contributor

Bug description
Selector adapter description:

A function or the name of the function that transforms an element of lov into a tuple(id:str, label:Union[str,Icon]).
The default value is a function that returns the string representation of the lov element.

The second sentence directly contradicts the first. The docs also give the default value of lambda x: str(x).

See this code and the following questions:

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

aggregation_list = ["minute", "hour", "day", "week"]
selected_aggregation_selector_single = "minute"

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

with tgb.Page() as page:
    tgb.text("# list[str] LoV", mode="md")
    tgb.selector("{selected_aggregation_selector_single}", lov="{aggregation_list}", adapter=aggregation_adapter, value_by_id=False)
    tgb.text("{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)

Question 1 - Purpose of first element in adapter return value

I expected that maybe the selection would be uppercase, since it is the id of the adapter return value, but it isn't. Is it used some other way?

Apparently, using return (aggregation.upper(), aggregation.title(), aggregation.upper()) achieves the functionality I just described -- the selector value would be uppercase. Assuming this is a bug, I would rather the application terminate with a TypeError -- it's a mistake for the app dev to fix rather than to fail gracefully.

Question 2 - value_by_id

Should value_by_id be impacted by the adapter? Description:

If False, the selection value (in value) is the selected element in lov. If set to True, then value is set to the id of the selected element in lov.

From this description, I infer that the selection value (in value) is always going to be from the lov, i.e. list[<this>: str] or list[tuple[<this>: str, str | Icon] (excl. the Enum case).

This makes sense and is fine by me, so should adapter simply have a return type of str | Icon for the entry label?

Expected change

  1. Clarify purpose of adapter returning tuple. Type signature in docs give the impression that it could be used to change the bound value
  2. Decide if adapter should even be able to change the bound value (in "value") of the selector
@arcanaxion arcanaxion added the 💥Malfunction Addresses an identified problem. label Jan 3, 2025
@arcanaxion
Copy link
Contributor Author

Also, why do the docs say the type for lov is dict[str, Any]?

@jrobinAV jrobinAV added 🆘 Help wanted Open to participation from the community 🟨 Priority: Medium Not blocking but should be fixed soon 📄 Documentation Internal or public documentation 🖰 GUI Related to GUI labels Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📄 Documentation Internal or public documentation 🖰 GUI Related to GUI 🆘 Help wanted Open to participation from the community 💥Malfunction Addresses an identified problem. 🟨 Priority: Medium Not blocking but should be fixed soon
Projects
None yet
Development

No branches or pull requests

2 participants