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

fallback to ObjectVar in guess_type #3911

Conversation

benedikt-bartscher
Copy link
Contributor

@benedikt-bartscher benedikt-bartscher commented Sep 11, 2024

ImmutableVar.__getattr__ does not allow attribute access and guess_type does not guess ObjectVar for arbitrary objects. In our codebases, this results in a lot var.to(ObjectVar)s
this pr currently just adds a fallback. Maybe we can instead check if there is a serializer which serializes to dict?

@benedikt-bartscher benedikt-bartscher marked this pull request as ready for review September 11, 2024 19:54
Merge remote-tracking branch 'upstream/main' into guess-type-fallback
Merge remote-tracking branch 'upstream/main' into guess-type-fallback
Copy link
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adhami3310 what do you think of this one

tests/components/datadisplay/test_datatable.py Outdated Show resolved Hide resolved
@adhami3310
Copy link
Member

I'm a bit concerned here on the general case, for example, a union of an int and str might be an ObjectVar now. It feels, impure, but maybe practicality trumps correctness.

To engage more, why were you creating so many ObjectVar? Can we find a better adjustment to the API to allow this behavior be simpler as an alternative?

@benedikt-bartscher
Copy link
Contributor Author

To engage more, why were you creating so many ObjectVar? Can we find a better adjustment to the API to allow this behavior be simpler as an alternative?

Because we often write serializers for existing objects and just want to use them in the frontend without creating an additional rx.Base "proxy class".
We could probably add some kind of marker which marks all classes which should be ObjectVars. Otherwise maybe follow my original idea from the PR description and only fallback to ObjectVar if there is a serializer registerd which serializes to dict/json.

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.

3 participants