-
Notifications
You must be signed in to change notification settings - Fork 1k
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
contextlib: (re) add ExitStack? #49
Comments
I'd say it was removed simply because it didn't run out-of-the-box, and there was no need at the time to get it working. |
What seems as "removal" is actually following guidelines of https://github.com/micropython/micropython-lib/wiki/ContributorGuidelines ("When porting a module from CPython or other 3rd-party source" section). So, it wasn't removed, it just "wasn't added" to micropython-lib by the original submitter. |
Some random collection of comments regarding ExitStack:
Actually, I'd like to keep it minimal, and current contextlib.py is already too big, and full of comments and stuff. Solve-it-all solution would be to create ucontexlib.py (in the own module of course), and let it be truly minimal (e.g. start with just contextmanager). Then contextlib.py can be fully CPython compatible (either importing ucontexlib, or just being completely separate, depending on what's easier to maintain). This still leaves questions of why you use "slightly modified" version, instead of verbatim CPython version, and how to be about tests (as I mentioned, if something tries to be ~fully CPython compatible, it should also take tests from CPython and pass them). So, if you'd be willing to resolve all these issues, it would be very welcome! |
Hi, Starting from @pfalcon's last question: ExitStack code from CPython does not run verbatim see dxxb@9c492ee for the (very limited) changes I made. Yes, ExitStack is large and I didn't know about it either until I went looking for the best way to enter context managers conditionally see the discussion at http://bugs.python.org/issue13585. I agree we should go with your solve-it-all solution. I would submit patches to spin off a ucontextlib.py module with just contextmanager and make the current contextlib depend on it then add ExitStack. CPython tests are an issue, they will not work with uPy because they rely an exception instance to have a context attribute and use it like this (taken from https://hg.python.org/cpython/file/3.4/Lib/test/test_contextlib.py)
|
I see that the only changes (sans white space) are to accommodate the fact that uPy doesn't support setting attributes on functions. I've come across this before trying to port some things. It's pretty expensive to allow writable attributes on functions.... |
@dxxb : Sounds good, thanks! |
There is another couple of context managers in contextlib that I would find useful when using unittest on my target: redirect_stdout() and redirect_stderr(). I am currently overriding the print() builtin and writing to an IOString but I'd rather redirect std[out, err]. However, sys.stdout cannot be set. Would it be acceptable to make sys.stdout writable for the unix port? |
I believe I worked on that, but can't see results of that work. Yes, it should be possible to override stdout/stderr on unix. If you think you can figure the machinery needed for this, you're welcome to look into that. |
Judging by this comment I need to look into making stdin/stdout/stderr r/w in modsys.c. I'll give it a shot. |
This is definitely a feature to add, but it's hard. The only way I can think to do it is something similar to MICROPY_CAN_OVERRIDE_BUILTINS, in objmodule.c. |
To give some context to all readers, I already proposed @dpgeorge to generalized MICROPY_CAN_OVERRIDE_BUILTINS support to any module, and he said he didn't see that having a good benefits/drawbacks ratio. |
Ok, I dug out my WIP in a stash and post it here for reference: pfalcon/pycopy@38300fe . As can be seen I do implement override support for any module. And sys.stdout is actually a hard case, as we store references to it in internal data structures. So, patch adds support for "module .setattr callbacks" to be able to handle such cases. |
Thank you @pfalcon, I am going to look into this as soon as I have the chance (probably next week). |
@pfalcon that patch is quite nice! Just a few issues with root pointers. |
It has been 9 years since the last comment. Is this still relevant? |
ExitStack was added in 5557382 from CPython 3.4.2 and subsequently removed in 92ef77c maybe because it wasn't compatible with uPy out of the box.
I am using a slightly modified ExitStack taken from CPython lib for some unittests which need to run on both my development host and on the target device and I am wondering if there is any interest in having ExitStack added to contextlib. Thanks
The text was updated successfully, but these errors were encountered: