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

delattr: use python code to add back missing builtin from cpython? #1251

Open
TG-Techie opened this issue Oct 8, 2018 · 10 comments
Open

delattr: use python code to add back missing builtin from cpython? #1251

TG-Techie opened this issue Oct 8, 2018 · 10 comments
Labels
cpython api modules from cpython enhancement
Milestone

Comments

@TG-Techie
Copy link

dear developers,
I am a huge fan of cp and am loving using it!

please hear me out:
was making a program loader and bufferer for my gui/user_thing by making classes and after asking around why I was having a problem Dan Halbert pointed out that delattr isn't in cp.

however with Mr.Halbert's help and a pile of dead bugs, I mean debugs :-), I found this works:
def delattr(target,name): exec('del str_target.'+name, {'str_target':target})
and am using it

and if you check the cpython page about builtin functions it says:
"For example, delattr(x, 'foobar') is equivalent to del x.foobar."
see: https://docs.python.org/3/library/functions.html#delattr

I would love to investigate implementing this at a c level or a python level!? (if small size # of bytes)
is that "kosher" to ask to do?
if it is okay for my to try this can i ask where the builtins are? I looked through repo but could not find it. in tools? (also tried finding getattr, search did not yield its location)

thanks for considering it!,
~TG-Techie

@dhalbert
Copy link
Collaborator

dhalbert commented Oct 8, 2018

The presence or absence of delattr() is determined by MPY_CPYTHON_COMPAT, which turns a lot of things on and off, costing about 800 bytes in the build. We can refactor this to break that big on/off switch into several smaller ones. The implementation of delattr() is quite small in the current py/modbuiltins.c

@TG-Techie
Copy link
Author

checking out: py/modbuiltins.c

@TG-Techie
Copy link
Author

I think I am missing a good bit of how cp is organised.
why is py/modbuiltins.c not implemented normally if it is in py?
best place to learn?

@dhalbert
Copy link
Collaborator

dhalbert commented Oct 8, 2018

The presence of delattr is controlled by MICROPY_CPYTHON_COMPAT

    #if MICROPY_CPYTHON_COMPAT
    { MP_ROM_QSTR(MP_QSTR_delattr), MP_ROM_PTR(&mp_builtin_delattr_obj) },
    #endif
#if MICROPY_CPYTHON_COMPAT
STATIC mp_obj_t mp_builtin_delattr(mp_obj_t base, mp_obj_t attr) {
    return mp_builtin_setattr(base, attr, MP_OBJ_NULL);
}
MP_DEFINE_CONST_FUN_OBJ_2(mp_builtin_delattr_obj, mp_builtin_delattr);
#endif

We could control that instead with a new MICROPY_PY_DELATTR flag. But note that, confusingly, there's already a MICROPY_PY_DELATTR_SETATTR, which implements __delattr__ and __setattr__, which are hook functions that get called when an attribute is deleted or set. So maybe we need a clearer new flag name.

@dhalbert
Copy link
Collaborator

dhalbert commented Oct 8, 2018

Or maybe we should just remove the #if, since I'm not sure why delattr() is conditionalized but getattr(), setattr(), and hasattr() are not.

@TG-Techie
Copy link
Author

Mr. Hablert, (Dan or Mr.Halbert)as more of a user than developer, hopefully just for nowcrosses fingers, I did find it is confusing that three are included yet the other isn't

@tannewt
Copy link
Member

tannewt commented Oct 8, 2018

@TG-Techie Do you have a link to your code? I'm surprised you need to use the attr functions directly.

@tannewt tannewt added this to the Long term milestone Oct 8, 2018
@tannewt
Copy link
Member

tannewt commented Aug 11, 2021

What do we want done here?

@tannewt tannewt added the cpython api modules from cpython label Aug 11, 2021
@dhalbert
Copy link
Collaborator

dhalbert commented Aug 11, 2021

Is MICROPY_CPYTHON_COMPAT turned on for all boards now? I'd start there. I'm not sure whether turning it on will bloat the tiniest builds too much.

@tannewt
Copy link
Member

tannewt commented Aug 11, 2021

Nope, it looks like it's tied to FULL_BUILD. There isn't really anything to do for this specific issue. Do we want to have a new issue to enable CPYTHON_COMPAT on all boards?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpython api modules from cpython enhancement
Projects
None yet
Development

No branches or pull requests

3 participants