-
Notifications
You must be signed in to change notification settings - Fork 349
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
New wrapper for flags #2284
base: master
Are you sure you want to change the base?
New wrapper for flags #2284
Conversation
I'd really prefer to have a layered approach. libghdl should be just a ctype interface, and a more pythonic interface would be implemented on top of it. Could you at least keep the wrappers as globals in flags.py ? That would also keep backward compatibility. |
With my proposal, I would like to enhance what ctypes doesn't do well. They have implemented a descriptor interface, but introduced another level of value access by using
I didn't fully get this point.
My goal is to provide a clean and easy syntax for bare variables from libghdl, which ctype doesn't provide. No further overhead or abstraction is planned on pyGHDL.libghdl. I'll do further improvements like getting rid of the In an ideal world, I wouldn't implement this using a singletone + (instance) properties / descriptors. I would prefer to use class properties (like @Property + @classmethod). Unfortunately, this feature is broken in Python and it's even a vulnerability. I'm trying to push on fixing it in Python itself. If so, it won't be available before 3.12 as I don't expect back porting. |
With my proposal, I would like to enhance what ctypes doesn't do well. They have implemented a descriptor interface, but introduced another level of value access by using .value syntax.
I think I perfectly understand your proposal and I think this is a good idea.
Could you at least keep the wrappers as globals in flags.py
I didn't fully get this point.
Do you mean:
• keeping the old ctypes wrappers in parallel to the Flags class, OR
• providing aliases to the descriptors, so old names still work?
I would simply keep the wrappers as they are and just add the Flags() class. No need to move the wrappers inside the class.
And yes, I agree that getting rid of the () would also be nice!
|
7b6f88c
to
ce48a95
Compare
@Paebbels, can this be rebased and merged or does it need additional work? |
I can rebase, but there is additional work needed from my side. |
This PR proposes a solution to get rid of
flags_name.value = True
syntax and uses proper descriptor protocols for hiding the C-binding wrapper.Old:
New:
@tgingold if you like the idea, I would do these additional steps:
Originally started PR was #2105.