Make the @micropython.native decorator no-op if support isn't enabled #2282
Conversation
|
This has not be tested yet on hardware, I'll update when I'd had a chance to test it out - just wanted to start the discussion. |
|
What if we just make @micropython.native work this way? We could add a separate way python code could test to see if it is supported. |
|
I'm up for that but I didn't want to deviate from MicroPython's behavior
with something that uses the same name.
…On Wed, Nov 13, 2019, 4:58 PM Scott Shawcroft ***@***.***> wrote:
What if we just make @micropython.native work this way? We could add a
separate way python code could test to see if it is supported.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2282?email_source=notifications&email_token=AAB5I47YDKDQ5CCD3GBCW3DQTSPCXA5CNFSM4JMIYQ32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEAGPIA#issuecomment-553674656>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB5I47CMWCI2SVIZPTA2ALQTSPCXANCNFSM4JMIYQ3Q>
.
|
I'm not sure we have to be completely consistent with MicroPython here, since any code that uses this is not too likely to be shared without change between MicroPython and CircuitPython. We could also add |
|
Cool, if everyone is cool with deviating from MicroPython I'll just change this to make We could also make it Making |
|
+1 deviating. |
|
Sounds bueno to me |
theacodes
force-pushed
the
native-if-available
branch
from
0ac83bc to
4d49c43
Compare
theacodes
changed the title
|
Okay, I've made this where (Updated the PR title as well) |
|
Okay, tested on real hardware and all seems well. Unfortunately, it looks like the added |
|
Sit tight. I'm freeing up some space in pIRkey with PR #2328. |
|
Rad
…On Tue, Nov 26, 2019 at 9:38 AM Scott Shawcroft ***@***.***> wrote:
Sit tight. I'm freeing up some space in pIRkey with PR #2328
<#2328>.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2282?email_source=notifications&email_token=AAB5I44SELGALL7UTG3J7M3QVVNKTA5CNFSM4JMIYQ32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFG3FYA#issuecomment-558740192>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB5I43SRYCNXSVFDDPLRDDQVVNKTANCNFSM4JMIYQ3Q>
.
|
When adding the ability for boards to turn on the `@micropython.native`, `viper`, and `asm_thumb` decorators it was pointed out that it's somewhat awkward to write libraries and drivers that can take advantage of this since the decorators raise `SyntaxErrors` if they aren't enabled. In the case of `viper` and `asm_thumb` this behavior makes sense as they require writing non-normative code. Drivers could have a normal and viper/thumb implementation and implement them as such:
```python
try:
import _viper_impl as _impl
except SyntaxError:
import _python_impl as _impl
def do_thing():
return _impl.do_thing()
```
For `native`, however, this behavior and the pattern to work around it is less than ideal. Since `native` code should also be valid Python code (although not necessarily the other way around) using the pattern above means *duplicating* the Python implementation and adding `@micropython.native` in the code. This is an unnecessary maintenance burden.
This commit *modifies* the behavior of the `@micropython.native` decorator. On boards with `CIRCUITPY_ENABLE_MPY_NATIVE` turned on it operates as usual. On boards with it turned off it does *nothing*- it doesn't raise a `SyntaxError` and doesn't apply optimizations. This means we can write our drivers/libraries once and take advantage of speedups on boards where they are enabled.
theacodes
force-pushed
the
native-if-available
branch
from
4d49c43 to
84e1d7f
Compare
|
Rebased to pick up the |
|
🤞 |
|
Tests pass, yay! Are there any other concerns or is this good to merge? |
When adding the ability for boards to turn on the
@micropython.native,viper, andasm_thumbdecorators it was pointed out that it's somewhat awkward to write libraries and drivers that can take advantage of this since the decorators raiseSyntaxErrorsif they aren't enabled. In the case ofviperandasm_thumbthis behavior makes sense as they require writing non-normative code. Drivers could have a normal and viper/thumb implementation and implement them as such:For
native, however, this behavior and the pattern to work around it is less than ideal. Sincenativecode should also be valid Python code (although not necessarily the other way around) using the pattern above means duplicating the Python implementation and adding@micropython.nativein the code. This is an unnecessary maintenance burden.So, this PR adds a new decorator:
@micropython.native_if_available. On boards withCIRCUITPY_ENABLE_MPY_NATIVEturned on it operates exactly like@micropython.native. On boards with it turned off it does nothing- it doesn't raise aSyntaxErrorand doesn't apply optimizations. This means we can write our drivers/libraries once and take advantage of speedups on boards where they are enabled.