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

ObjectVar __getitem__ typing issues #4380

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

benedikt-bartscher
Copy link
Contributor

@benedikt-bartscher benedikt-bartscher commented Nov 13, 2024

@adhami3310 do you have any idea on how to fix the typing test?

reflex/tests/units/vars/test_object.py:112:5 - error: "__getitem__" method not defined on type "ObjectItemOperation" (reportGeneralTypeIssues)

@adhami3310
Copy link
Member

ObjectItemOperation is essentially just a Var with no typing. You can generally call .to(< the type it should be> on it.

@adhami3310
Copy link
Member

I think in the example above it could lose the typing ? I can take a look into that.

@benedikt-bartscher
Copy link
Contributor Author

benedikt-bartscher commented Nov 13, 2024

I think in the example above it could lose the typing ? I can take a look into that.

Reflex always supported accessing vars in that way, so I think the general goal should be to add full typing support for this. (Edit: Unless you plan to migrate away from this syntax)
However, we could lose the typing as a mitigation. But I would not put too much effort into that, we can use # pyright ignore annotations until reflex supports it.

@adhami3310
Copy link
Member

by lose typing i meant runtime typing, reflex should maintain runtime typing as long as possible to limit the need for .to, but static typing is a different question, and generally difficult to get right with object vars

@adhami3310
Copy link
Member

i believe before, all vars supported [], but right now we're moving away from that, the issue in this case is that static typing doesn't pick up on that field is an object var, as far as i know, it seems impossible to provide static typing that would not lose the context here without returning Any

@benedikt-bartscher
Copy link
Contributor Author

Sounds like you are hitting some of python's typing limits? :/

I have just added another test (1384514) which is quite interesting. dict[str, str] works but dict[str, bool] does not. This sounds like it should be fixable.

@adhami3310
Copy link
Member

Sounds like you are hitting some of python's typing limits? :/

i hit them very long time ago 😅 the issues are not particularly fixable.

dict[str, str] works but dict[str, bool] does not

yea! i will take a look once i'm done with another PR that overhauls functions first

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.

2 participants