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

rx.foreach Union support #1814

Closed
jpbaltazar opened this issue Sep 14, 2023 · 2 comments · Fixed by #2010
Closed

rx.foreach Union support #1814

jpbaltazar opened this issue Sep 14, 2023 · 2 comments · Fixed by #2010
Labels
feature request A feature you wanted added to reflex

Comments

@jpbaltazar
Copy link

jpbaltazar commented Sep 14, 2023

In my app I'm trying to use an union to hold slightly different classes (common base members but extra members) and using rx.foreach to try to display those entities. If I specify the union, the foreach component won't run, citing :

AttributeError: The State var `_` has no attribute 'MY_ATTRIBUTE' or may have been annotated wrongly.

I've tried to simply make these classes inherit from a single class but I of course cannot access the other members not from the class.

Could I request support for unions for rx.foreach? Would it be hard to implement?
Edit: One option would be to provide a way of specifying a render_fn per type and a default one?

My example setup:

from pydantic import Field
import reflex as rx

class Reaction1(rx.Model):
    reaction: str = Field("instant", const=True)

class Reaction2(rx.Model):
    reaction: str = Field("delay", const=True)
    delay: int = Field(default=20)

ReactionTypes = Reaction1 | Reaction2

reaction_list: list[ReactionTypes] = [
    Reaction1(**{"reaction": "instant"}),
    Reaction2(**{"reaction": "delay", "delay": 40}),
]

class State(rx.State):

    @rx.var
    def reaction_list(self) -> list[ReactionTypes]:
        return reaction_list
    
def show_entry(entry: ReactionTypes) -> rx.Component:
    if isinstance(entry, Reaction2):
        return rx.hstack(
            rx.text(entry.reaction),
            rx.text(f"delay: {entry.delay}"),
        )
    else:
        return rx.text(entry.reaction)
        
def index() -> rx.Component:
    return rx.vstack(
        rx.foreach(
            State.reaction_list,
            show_entry,
        ),
    )


app = rx.App()
app.add_page(index)
app.compile()
@jpbaltazar jpbaltazar changed the title rx.foreach union support rx.foreach Union support Sep 14, 2023
@Lendemor Lendemor added the feature request A feature you wanted added to reflex label Sep 14, 2023
@masenf
Copy link
Collaborator

masenf commented Sep 14, 2023

if isinstance(entry, Reaction2):

This line is problematic because entry is always going to be a BaseVar, not an actual instance of the class it originated from.

The code that comprises the frontend is compiled into javascript code that runs in the browser, so any python conditions and type differentiation must be workable at compile time, not runtime. All runtime logic that needs to be in python must be in event handlers and computed vars in the state.

Going back to your original request though, I think it wouldn't be too hard to support union types in the foreach, but you would need your own way of differentiating the types post-serialization if you want to work with them on the frontend.

Consider a base class with reaction_type as a field that would get serialized and sent to the frontend; then you could use rx.cond to conditionally render components based on the type.

Until union support is added, you might try to define a ReactionFieldSuperSet class which basically includes all of the possible fields and annotate the computed var as returning that type. This will kind of fool the type checker into allowing attribute access to any field valid on any Model, but then you must take care to only access fields appropriate for the given object type.

@jpbaltazar
Copy link
Author

I think I understood the runtime vs frontend part, thank you.

Ok that's more or less the solution I've ended up at, making a class with a label that identifies the type.

Thank you for considering adding union support!

masenf added a commit that referenced this issue Oct 20, 2023
Fix #1814

* Remove py3.7 workarounds, use get_args and get_origin directly
* Move `can_access_attribute` to types and call recursively for union types
* Extend `is_valid_var_type` to allow Union where all args are valid var types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A feature you wanted added to reflex
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants