-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
Add "u" prefix in repr() of usys objects. #7709
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
This commit changes >>> repr(usys.stdout) '<io.TextIOWrapper 1>' to >>> repr(usys.stdout) '<uio.TextIOWrapper 1>' in order to match the module it is imported from.
This was done for some comments already, but not all of them.
Can you explain the reason for this change? I.e. why is it preferred to 'break' things the other way? I mean it used to be
which will now be
so that's still a mismatch, just different. |
Thanks @laurensvalk In #5377 (which was recently merged and undoes the documentation part of #6164), I made the case that for most users the module are called "foo" not "ufoo". The "u" prefix is an implementation detail that allows for the module to be extended by Python (and allowing this was the part purpose of @stinos' #6164). With the exception of some leftovers in extmod, all the modules are defined in modfoo.c, the "u" prefix is only applied at module registration. Some of the tests need to use I think the guiding principle should be that ideally most users of MicroPython are never aware of the "u" prefix. They should always write "import foo" in their Python code (with the exception of
So I think I would argue that the current handling of repr is correct, but I agree this is an interesting case! Do you have more background from pybricks? |
This is the current behavior:
It seemed reasonable to do the same for its contents, so |
And regarding the change to Lines 1142 to 1150 in 145fede
|
This is confusing... and inconsistent, e.g. sys is sys, but most have the u prefix. I think I'd argue that they should all be "foo" not "ufoo". I don't think fixing this would be a backwards compatibility issue...? You can still tell the difference between foo.py vs builtin-foo with the existence of foo.path. Note: everything is registered as ufoo (and it should stay this way in order to make weak links work), it's just name that should change, for the following modules:
That's 20 bytes! (maybe) Will be interested to see what @dpgeorge thinks!
Yeah I think this second case "ucollections.deque" should be "collections.deque". |
I think In general I agree that |
I originally opened this PR to address the inconsistency, not to argue one way or the other, but since you ask, here it goes: I see modules like
I would personally argue the opposite. In our experience with beginning users, it's a lot easier to say, "MicroPython works exactly like this", as opposed to "it's like Python, except for these 100+ differences". This approach does require such exact documentation for MicroPython modules, which is what we're currently working on. It's currently somewhat specific to our target boards (LEGO hubs), but in principle it can be adapted for MicroPython more generally if there's any interest. |
I might be looking at it from a different angle, but from my point of view using not
|
The intent of having the "u" prefix was that the only place that should need to use this prefix is a module which is building ontop of one of the u modules to add missing functionality. i.e. you're creating an os module which extends the functionality of the uos module. In this case, the os.py needs to import uos. In all other cases, the intent was that the user would just use |
For what it's worth as far as the original PR is concerned, CPython has this behavior:
Other than using |
I'll also add that this particular PR isn't really all that important. The module name discussion itself is more relevant, but it seems that this is being discussed in various other places already.
Edit: I see now that #5377 removes |
This is superseded by #9018, so we can probably close this. |
This follows up on @stinos' #6164.
This updates
to
in order to match the module it is imported from.
I've also updated a few comments in mpconfig to add the prefix.
EDIT: Fix
tests/extmod/vfs_fat_fileio1.py
not passing.