-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
py/obj: Add support for __float__ converter function. #8857
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
Conversation
One point of discussion; if We looked at adding the conversion fallback behaviour to improve Python compatibility but found that it added enough complexity that we were questioning it's value. Further, the same behaviour (call I guess the question is should we bother? It seems like an esoteric corner of the language... |
py/obj.c
Outdated
@@ -67,10 +67,10 @@ const mp_obj_type_t *MICROPY_WRAP_MP_OBJ_GET_TYPE(mp_obj_get_type)(mp_const_obj_ | |||
} else if (mp_obj_is_obj(o_in)) { | |||
const mp_obj_base_t *o = MP_OBJ_TO_PTR(o_in); | |||
return o->type; | |||
#if MICROPY_PY_BUILTINS_FLOAT | |||
#if MICROPY_PY_BUILTINS_FLOAT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to not reformat these #'s ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah whoops, the first ~5 times I pushed this branch I undid all the auto-codeformat on these lines, must have missed that on the current / last push. Not sure why my uncrustify wants to do this here :-(
Did you consider using the same approach as those existing ones, using |
I briefly did but saw the comment here about if forcing a bytecode update: Line 66 in 07cae91
I presume this was is more efficient? I haven't followed that code path enough to know how it works yet. That being said I guess "not updating the bytecode version" isn't a great excuse for taking shortcuts though, consistency with existing code is important... update: geez I am blind sometimes... that's what I get for rushing I guess. Only a few lines below that comment I linked above is the follow up comment |
Codecov Report
@@ Coverage Diff @@
## master #8857 +/- ##
=======================================
Coverage 98.31% 98.31%
=======================================
Files 157 157
Lines 20349 20365 +16
=======================================
+ Hits 20006 20022 +16
Misses 343 343
Continue to review full report at Codecov.
|
Looking closer at the existing
Is the point of all these functions that they return a small int / non-gc object? As such float and complex wouldn't fit here as they return larger objects? |
No. It's just that the |
Superseded by #8899. |
I've run into a number of situation where I'm using a custom class that's intended to wrap an integer or float value, eg. micropython/micropython-lib#502
It's confusing that other dunder converter functions like
__int__
and__str__
work but not__float__
. This PR resolves this, though in a different way to these other functions.Another similar question was raised here: https://forum.micropython.org/viewtopic.php?f=2&t=6467