top: Replace `umodule` with `module` everywhere. by jimmo · Pull Request #9069 · micropython/micropython

jimmo · 2022-08-18T07:35:17Z

Follow-up to #9018 -- this is the groundwork for doing the renaming of all built-in modules as well as renaming files and variables. I have always found the "u" prefix confusing, and although weak links provide a workaround there's still weirdness like the output of help('modules') etc. See #9018 for more but I strongly feel we should move in a direction of CPython compatibility and consistency.

Updated from an earlier version of this PR. This is now no longer a breaking change. It continues the decision in #11456 but makes the implementation of that more explicit by allowing modules to be defined as "extensible" or (default) "non-extensible" rather than inferring this from the u-prefix.

In summary:

There is now no longer any umodule naming anywhere in the code (file names, qstrs, symbols, macros).
A module can now be registered either as MP_REGISTER_MODULE or MP_REGISTER_EXTENSIBLE_MODULE. An extensible module can optionally be overridden from the filesystem, whereas a regular one will always bypass the filesystem. (An extensible module is what would have previously had a u prefix, e.g. uos).
In order to retain backwards compatibility, importing an extensible module with it's u prefix will still work (this is kind of the inverse of weak links and is unconditionally enabled), and will bypass the filesystem. New code should prefer to use the sys.path mechanism introduced in py/builtinimport: Allow built-in modules to be packages (v3) #11456 instead (and hopefully we can deprecate the special u-handling later).
sys.path is now assignable (using the same "mutable sys" mechansim as sys.ps1 etc).

Reducing the size of the QSTRs has a small code size decrease (~100 bytes), but this is offset by the extra u-prefix handling and the split built-in modules maps, so there's a net negative code size increase (+12 bytes on PYBV11, +84 bytes on bare-arm). We could make the u-prefix handling conditional, but previously the minimal ports only supported "import umodule" so we should keep that working. However, this PR also removes the timeq module which gives a net saving around -850 bytes on PYBV11.

This work was funded through GitHub Sponsors.

robert-hh · 2022-08-18T11:36:25Z

@jimmo

due to a limitation of the bash scripts I used to automate most of this. I will fix it up of course if we decide to merge.

Looking at the massive number files that have been changed this looks like needing a lot of changes.
Since I keep two ports alive until they get eventually merged (at least one of them), is is possible to provide the bash script you used for changing the sources?

jimmo · 2022-08-18T14:11:51Z

I have updated the tests to translate

try:
  import foo
except:
  import foo

to just import foo.

Since I keep two ports alive until they get eventually merged (at least one of them), is is possible to provide the bash script you used for changing the sources?

@robert-hh ... I did this as a series of (many!) one-liners. There was a bit of trial and error and some manual fixups.

Almost all of the changes are outside the ports, but in terms of rebasing a port, the one-liner below should get you a long way though if you run this on your branch before doing the rebase.

umod.txt

cd ports/samd
# See umod.txt attached to this comment.
cat ~/mpy/umod.txt | while read mod; do git grep -l $mod | xargs sed -i -r "s/(^|[^a-z])$mod($|[^a-z])/\1${mod:1}\2/g"; done
# ignore the no input files messages

robert-hh · 2022-08-18T14:19:00Z

Thanks. I will try that when the time comes.

codecov · 2023-06-01T17:13:56Z

Codecov Report

Merging #9069 (a1fbb19) into master (d080d42) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #9069      +/-   ##
==========================================
- Coverage   98.40%   98.40%   -0.01%     
==========================================
  Files         156      155       -1     
  Lines       20622    20539      -83     
==========================================
- Hits        20294    20211      -83     
  Misses        328      328

Impacted Files	Coverage Δ
extmod/modbinascii.c	`100.00% <ø> (ø)`
extmod/moductypes.c	`97.83% <ø> (ø)`
extmod/modwebsocket.c	`89.76% <ø> (ø)`
extmod/vfs_posix.c	`93.49% <ø> (ø)`
extmod/vfs_posix_file.c	`92.22% <ø> (ø)`
py/builtin.h	`100.00% <ø> (ø)`
py/moderrno.c	`100.00% <ø> (ø)`
py/modio.c	`98.52% <ø> (ø)`
py/modstruct.c	`100.00% <ø> (ø)`
py/obj.h	`100.00% <ø> (ø)`
... and 28 more

github-actions · 2023-06-01T17:17:09Z

Code size report:

   bare-arm:   +84 +0.149% 
minimal x86:  +352 +0.189% [incl +32(data)]
   unix x64: -2280 -0.286% standard[incl -416(data) -24(bss)]
      stm32:  -816 -0.208% PYBV10[incl -16(bss)]
        rp2:   -36 -0.011% PICO[incl -12(bss)]

jimmo · 2023-06-01T17:31:48Z

Now that #11456 is merged I have rebased & updated this.

For the time being I have not added a replacement for weak links. We need a way to annotate built-in modules as extensible-from-the-filesystem or not, and then match non-extensible ones at the start of import and extensible at the end. Also a mechanism to allow backwards-compatible import ufoo.

jimmo · 2023-06-02T03:32:40Z

Added a mechanism to replace the former weak link system and make explicit the behavior defined in #11456 about extensible vs non-extensible modules. Updated the PR description with details.

jimmo · 2023-06-02T14:02:29Z

OK all tests passing at last!

@dlech @stinos @andrewleech I think this should alleviate the concerns from #9018. My intention with this PR is now that the only user-visible difference is the output of help("modules") and what you see if you print a module object (i.e. <module 'os'> rather than <module 'uos'>). All existing code should maintain the same behavior.

For people working in the codebase, the main change is the file and symbol renaming, but overall I believe this improves consistency and clarity.

dpgeorge · 2023-06-08T07:50:37Z

I have finished a full and detailed review of this PR. There are many changes done in the commits here, but a lot of them are simple renames, and most of those are in boilerplate code (eg defining modules and functions).

It's definitely great to see things like the tests simplified to just do import time, instead of the existing try import utime as time; except ImportError: import time.

Overall the changes here are a really good step forward. It simplifies module naming, makes MicroPython behave more like CPython, still allows extensibility of built-ins.

This renames the builtin-modules, such that help('modules') and printing the module object will show "module" rather than "umodule". This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

In order to keep "import umodule" working, the existing mechanism is replaced with a simple fallback to drop the "u". This makes importing of built-ins no longer touch the filesystem, which makes a typical built-in import take ~0.15ms rather than 3-5ms. (Weak links were added in c14a816) This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

This work was funded through GitHub Sponsors. Also updates #includes. Signed-off-by: Jim Mussared <[email protected]>

Updates any includes, and references from Makefiles/CMake. This essentially reverts what was done long ago in commit 136b5cb This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

Signed-off-by: Jim Mussared <[email protected]>

This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

This replaces the previous QSTR_null entry in the globals dict which could leak out to Python (e.g. via iteration of mod.__dict__) and could lead to crashes. It results in smaller code size at the expense of turning a lookup into a loop, but the list it is looping over likely only contains one or two elements. To allow a module to register its custom attr function it can use the new `MP_REGISTER_MODULE_DELEGATION` macro. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

When compiling mpy-cross, there is no `sys` module, and so there will be no entries in the `mp_builtin_module_delegation_table`. MSVC doesn't like this, so instead pretend as if the feature isn't enabled at all. Signed-off-by: Jim Mussared <[email protected]>

Otherwise you can get into the confusing state where e.g. sys.ps1 is enabled in config (via `MICROPY_PY_SYS_PS1_PS2`) but still doesn't actually get enabled. Also verify that the required delegation options are enabled in modsys.c. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

Previously sys.path could be modified by append/pop or slice assignment. This allows `sys.path = [...]`, which can be simpler in many cases, but also improves CPython compatibility. It also allows sys.path to be set to a tuple which means that you can clear sys.path (e.g. temporarily) with no allocations. This also makes sys.path (and sys.argv for consistency) able to be disabled via mpconfig. The unix port (and upytesthelper) require them, so they explicitly verify that they're enabled. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

Make tests run in an isolated environment (i.e. `import io` would otherwise get the `tests/io` directory). This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

Applies to drivers/examples/extmod/port-modules/tools. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

- Update guide for extending built-in modules. - Remove any last trace of umodule in other docs. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

This is a MicroPython-specific module that existed to support the old version of uasyncio. It's undocumented and not enabled on all ports and takes up code size unnecessarily. Signed-off-by: Jim Mussared <[email protected]>

dpgeorge · 2023-06-08T12:43:34Z

Thanks @jimmo for all the hard work on this one.

for micropython/micropython#9069 handle MICROPY_PY_RANDOM_SEED_INIT_FUNC not being defined

jimmo force-pushed the remove-umodule branch from c749022 to 2009e87 Compare August 18, 2022 07:38

jimmo force-pushed the remove-umodule branch 5 times, most recently from af00296 to 7e6a9d1 Compare August 18, 2022 13:57

jimmo force-pushed the remove-umodule branch 2 times, most recently from e04e663 to 0b9e093 Compare August 19, 2022 13:11

jimmo mentioned this pull request Oct 3, 2022

tools/pyboard.py tries to "import uos" on repl client -- there is no uos. #9496

Closed

jimmo mentioned this pull request May 10, 2023

py/builtinimport: Allow built-in modules to be packages (v3) #11456

Merged

jimmo force-pushed the remove-umodule branch from 0b9e093 to 3cdfd14 Compare June 1, 2023 17:11

jimmo force-pushed the remove-umodule branch 2 times, most recently from 812ca18 to 3d083a1 Compare June 1, 2023 17:30

jimmo force-pushed the remove-umodule branch from 3d083a1 to 8a17042 Compare June 2, 2023 03:30

jimmo force-pushed the remove-umodule branch 7 times, most recently from ca40556 to 1bd7804 Compare June 2, 2023 13:56

jimmo mentioned this pull request Jun 2, 2023

RFC: Built-in module extending and removing weak links / umodules #9018

Closed

dpgeorge reviewed Jun 8, 2023

View reviewed changes

Comment thread docs/library/index.rst Outdated

jimmo force-pushed the remove-umodule branch from e550aa1 to 677a6d5 Compare June 8, 2023 07:46

jimmo added 21 commits June 8, 2023 17:53

all: Rename MP_QSTR_umodule to MP_QSTR_module everywhere.

30628d1

This renames the builtin-modules, such that help('modules') and printing the module object will show "module" rather than "umodule". This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

all: Rename mp_umodule*, mp_module_umodule* to remove the "u" prefix.

1bf2dcb

This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

all: Rename mod_umodule*, ^umodule* to remove the "u" prefix.

7f5d5c7

This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

all: Rename UMODULE to MODULE in preprocessor/Makefile vars.

f5f9edf

This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

all: Rename *umodule*.h to remove the "u" prefix.

0ceccd4

This work was funded through GitHub Sponsors. Also updates #includes. Signed-off-by: Jim Mussared <[email protected]>

all: Rename *umodule*.c to remove the "u" prefix.

45ac651

Updates any includes, and references from Makefiles/CMake. This essentially reverts what was done long ago in commit 136b5cb This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

py/makemoduledefs.py: Add a way to register extensible built-in modules.

24c02c4

Signed-off-by: Jim Mussared <[email protected]>

all: Use MP_REGISTER_EXTENSIBLE_MODULE for overrideable built-ins.

2eba98f

Signed-off-by: Jim Mussared <[email protected]>

examples/natmod: Rename umodule to module.

eb85f4d

This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

tests: Replace umodule with module everywhere.

4216bc7

This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

tests/run-multitests.py: Don't allow imports from the cwd.

1097174

Make tests run in an isolated environment (i.e. `import io` would otherwise get the `tests/io` directory). This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

tests/run-perfbench.py: Don't allow imports from the cwd.

339f02a

Make tests run in an isolated environment (i.e. `import io` would otherwise get the `tests/io` directory). This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

tests/run-natmodtests.py: Don't allow imports from the cwd.

9d7eac0

Make tests run in an isolated environment (i.e. `import io` would otherwise get the `tests/io` directory). This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

all: Replace all uses of umodule in Python code.

5fd042e

Applies to drivers/examples/extmod/port-modules/tools. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

docs/library/index: Update docs after umodule rename.

8211d56

- Update guide for extending built-in modules. - Remove any last trace of umodule in other docs. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>

extmod/modtimeq: Remove timeq module.

a1fbb19

This is a MicroPython-specific module that existed to support the old version of uasyncio. It's undocumented and not enabled on all ports and takes up code size unnecessarily. Signed-off-by: Jim Mussared <[email protected]>

jimmo force-pushed the remove-umodule branch from 677a6d5 to a1fbb19 Compare June 8, 2023 07:54

dpgeorge merged commit a1fbb19 into micropython:master Jun 8, 2023

jimmo mentioned this pull request Jun 8, 2023

all: Rename uasyncio to asyncio. #11740

Merged

peterzuger added a commit to peterzuger/hydrogen-micropython that referenced this pull request Jun 26, 2023

updated URANDOM to RANDOM

df6d5bf

for micropython/micropython#9069 handle MICROPY_PY_RANDOM_SEED_INIT_FUNC not being defined

dhalbert mentioned this pull request Aug 22, 2023

remove last uses of 'u' prefix adafruit/circuitpython#8318

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

top: Replace `umodule` with `module` everywhere.#9069

top: Replace `umodule` with `module` everywhere.#9069
dpgeorge merged 21 commits into
micropython:masterfrom
jimmo:remove-umodule

jimmo commented Aug 18, 2022 •

edited

Loading

Uh oh!

robert-hh commented Aug 18, 2022

Uh oh!

jimmo commented Aug 18, 2022

Uh oh!

robert-hh commented Aug 18, 2022

Uh oh!

codecov Bot commented Jun 1, 2023 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 1, 2023 •

edited

Loading

Uh oh!

jimmo commented Jun 1, 2023 •

edited

Loading

Uh oh!

jimmo commented Jun 2, 2023

Uh oh!

jimmo commented Jun 2, 2023

Uh oh!

Uh oh!

dpgeorge commented Jun 8, 2023

Uh oh!

dpgeorge commented Jun 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

jimmo commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robert-hh commented Aug 18, 2022

Uh oh!

jimmo commented Aug 18, 2022

Uh oh!

robert-hh commented Aug 18, 2022

Uh oh!

codecov Bot commented Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jimmo commented Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jimmo commented Jun 2, 2023

Uh oh!

jimmo commented Jun 2, 2023

Uh oh!

Uh oh!

dpgeorge commented Jun 8, 2023

Uh oh!

dpgeorge commented Jun 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jimmo commented Aug 18, 2022 •

edited

Loading

codecov Bot commented Jun 1, 2023 •

edited

Loading

github-actions Bot commented Jun 1, 2023 •

edited

Loading

jimmo commented Jun 1, 2023 •

edited

Loading