Enhance stub generation with improved type handling and deduplication logic#573
Enhance stub generation with improved type handling and deduplication logic#573Josverl wants to merge 1 commit into
Conversation
… logic Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com>
kdschlosser
left a comment
There was a problem hiding this comment.
It looks good for the most part. I know there are still remaining issue because your code changes have not addressed some of them but all in all it's not bad. There are 2 things that I made comments on that should be looked over.
I can tell you that there is an issue with how the code generator marries a function to a class so it is a method. That issue is in the C generator code and it would need to be corrected there and that correction should populate out to the stub generator to fix the problem there.
Another issue which only exists in the stub generator is the enumerations are not correct. Specifically the class structure for them.
You will also want to make sure that all of the widgets are subclasses of the obj class. This is important because it is how the organization is done in lvgl.
| ''' | ||
|
|
||
| # Use decorators for setter-only properties. | ||
| prop_set_template = '''\ |
There was a problem hiding this comment.
You cannot so it this way because there are items where set exists and get doesn't so having the template like this will cause an error if the getter is used. While it would still throw an error if a user tries to use the get when it doesn't exist the error message is not going to be a cryptic C style error.
There was a problem hiding this comment.
The previous style was just illegal syntax for a stub, so something needs to change
There was a problem hiding this comment.
How is doing this illegal syntax for a stub?????
class SomeClass:
def some_property(self, value):
pass
some_property = property(fset=some_property)
There was a problem hiding this comment.
from memory - its not allowed to assign values in a @dataclass
There was a problem hiding this comment.
after going back and forth with Claude AI we both came to the same conclusion. There is a bug in PEP 484. There is no mechanism to declare a setter only property in a stub file when there should be because setter only properties do exist and are perfectly legal code. By supplying a getter even with it explicitly typed as returning None an IDE is not going to indicate that there is no getter available.
it also appears that when I explicitly set the return type to None it doesn't work either...
and here you can see that it shows as being available as a getter even tho the return type is explicitly None.
It is a known issue without any solution to fix it. I don't see why it would be so hard to correct but apparently it is with no known way to fix it.
There was a problem hiding this comment.
cannot use NoReturn either. I tried that one as well. There is simply no way to properly type a write only property. As it turns out there is no way to type a read only property either and that is the one that has been directly discussed and PEP 767 is a proposal for fixing the read only part and not the write only part. There has been no discussion about a write only property.
There was a problem hiding this comment.
This is the best I can come up with:
Use in code
from lv_props import SomeClass
s = SomeClass()
# Valid setters and getters.
s.foo_both = 1.0
print(s.foo_both)
print(s.foo_get)
s.foo_set = 2.0
# Invalid setters and getters.
s.foo_get = 3.0 # Flagged correctly as an error.
reveal_type(s.foo_set) # Reveals type as None, which is correct.
print(s.foo_set) # Not flagged as an error, but raises NotImplementedError at runtime and shows on hover.# Stub
# Use decorators for setter-only properties.
class SomeClass:
@property
def foo_both(self) -> float: ...
@foo_both.setter
def foo_both(self, value: float) -> None: ...
@property
def foo_get(self) -> float: ...
@property
def foo_set(self) -> NotImplementedError: ...
@foo_set.setter
def foo_set(self, value: float) -> None: ...| generated_constant_names = set() | ||
|
|
||
| # Emit existing aliases as TypeAlias and deduplicate by name. | ||
| FIXED_ALIASES = [ |
There was a problem hiding this comment.
The code generator is written so that it will work on C code other than LVGL. That means we do not want to have LVGL specific markers if possible in the binding generator code. I know there are places where it can be seen in the code and that is out of absolute necessity. If there is a different way to handle aliases other than creating lookup tables it would be the preferred thing to do. The only time you should be hard coding anything that points specifically to anything LVGL related is if there is no other way.
There was a problem hiding this comment.
Should be possible to keep this dynamic, and still emit proper typeAliases.
|
Thanks, I think these can be solved, though i dont thing a 100% generic C++ to stub generator may be be impossible. |
|
it won't be 100% possible in all use cases but when using a lexer to parse the C code as the code generator does you can get very close. Using the ESPIDF as an example. the code generator can be used on portions of the ESP IDF so it can be accessed in MicroPython. I have personally used it to do that with the TWAI drivers. I am not the person that originally wrote the binding generator. It was written by the person that wrote the original LVGL MicroPython binding. It's not very well written, confusing and hard to understand what is happening. It's also very fragile and it doesn't generate the most efficient code with many layers of nesting that has been done. It is something I would love to do away with or even better would be to simplify it flattening the API so it is identical to what LVGL is instead of packing most of the functions into classes based on the type of the first parameter and the start of the function name needing to match the first part of a structure. What actually might be better to use instead of the code generator for creating the stub file is the script I wrote for LVGL that outputs the entire API as JSON. The script is packaged with LVGL and it would be easy to write the script to read the JSON to output the stub data. The other thing is the docstrings are also in the JSON output as well. |
Skipping steps, and gaining extra information, seems like worth the investigation. Also IIRC this was mentioned before. But i could not figure out how to fulfill the prerequisites at that time. If you have a pointer for that im happy to take a stab at this. |
|
If you look at the repo in the libs folder you will see lvgl there. You would need to perform a submodule init on the LVGL folder to pull it down to your clone. under LVGL there is a scripts folder and under that folder there is the gen_json folder. In the gen_json folder there is a requirements file that has the needed modules declared in it that are needed for it to run. you also have some external deps like doxygen and a C compiler that will need to be installed as well. There are 2 ways you can run the script. It can be run directly from a shell/command line and optionally it can be run by importing the module in a python script. You would want to run it from a python script so you can read the output from it and convert it to the needed output as a stub file. The script would be something along the lines of... Here is the API for the json data... https://github.com/lvgl/lvgl/blob/master/docs/src/integration/bindings/api_json.mdx |
Hi,
I took look at the
lvgl.pyithat is generated during build - and I found that it had a rather large number of errors ( 900+ IIRC)I have worked though them , and now it generates clean no errors at all on the few builds I tried.
There are still things to improve, but pyright is happy.
Fixes:
TypeAliases__futures__imports__init__return types@propertydecorators()🔮AI Assisted