'print' statement ignores 'sys.stdout' #39
Comments
|
Effectively, the sys module does this: The Print() function in the Go runtime writes directly to grumpy.Stdout. It does not get it from the sys module. So things work fine until you re-bind sys.stdout, as you've found. There are a couple ways to fix this:
In short, there's no great way to fix it at the moment, however, I think some avenues will open up once Grumpy is self-hosted. |
|
What about implementing the print statement as a pure-python function, and hook the golang Print function to the python one? I will need to reimplement the print stuff very soon anyway... |
|
If, for example, you define print() in __builtins__.py then you have the problem of accessing the __builtins__ Go package from within the grumpy runtime package, because they each would depend on each other. |
|
I am thinking about defining some _print_statement.py and hooking Print() Ideally, it should import only sys.stdout I did not yet took a look there to see what the Print() imports Il 23 apr 2017 1:18 PM, "Dylan Trotter" [email protected] ha
|
|
I think the fundamental issue you're doing to run into is circular
|
|
I just found the Print() function, and it should not rely on 'file' arg being a File anyway, otherwise "print" will never accept a duck-typed file-like, in my opinion. For now, my plan is to factor it out from builtins, even if it is always loaded Will draw some sketches on dead-trees to help with the circular dependencies. Maybe lazy-importing sys.Stdout, with a local cache to not penalize the most used case. I like it not, however. |
|
+1 to taking any object instead of a file. Looking forward to seeing what
|
|
Are there a test cases for Python scripts that start with |
|
@trotterdylan: Thinking in Print() check for 'sys' in SysModules and, if found, use 'sys.stdout'. If not found, fallback to Stdout as the 'sys' was not imported and sys.stdout is sure to be Stdout anyway. |
|
Although that solution should work (if sys was not imported, then there's no way for someone to override it from Python), it's not very satisfying IMO. It's pretty easy to imagine a scenario where this logic would diverge from CPython (probably involving messing around with sys.modules). Although these scenarios are far fetched, I'm more worried that this implementation divergence from CPython will cause other, more subtle problems. I think the right approach is to make the sys module "built in" in a similar way to CPython. A variation on option 1. above is to do exactly what CPython does and define the sys module within the runtime itself: https://github.com/python/cpython/blob/2.7/Python/sysmodule.c This would still require some changes to the import system to special case sys, but it's probably the best option we have available in the short term. |
|
@trotterdylan I definitely agree with your idea. If we need modules of python which are implemented by C-level API. At that case, We should implement it by Go also. |
|
I do not know if I like this path. I remember of news of some conference (could not find the reference, sorry) where people of different Python "runtimes" including CPython agreed that the best would be to write the stdlib in pure-python if possible, falling back to C-API only for serious speed needs or low-level access needs. In the long term, do you expect to be easier to maintain a pure-python sys or a Go sys? Another point is that, as we are borrowing PyPy and Ouroboros code, they could take it back and share the maintenance burden with us, but only if our modules are useful to him. For now, my view of the thing is to implement whatever possible in pure-python (will be transpiled to Go anyway...), not worrying with implementation divergence from CPython. The result could be simpler and easier to maintain. And more useful to other pals too (PyPy for exemple). |
|
Be clear that it is my philosophical perspective, more aligned with PyPy "view". But a view like "lets transpile CPython code to Go, whatever it is a good code or not" will sure work too, and will easily emulate the goods and bads there. Including maintenance ones. |
|
Please confirm your spirits. I can wait or can give a shot, even if to change it later. just confirm. |
|
I agree with the spirit of your argument, but our sys.py would not be portable Python anyway, since it has to use __go__ imports to function properly. The sys module is kind of special in that it is tightly integrated with the interpreter state (default encoding, sys.modules, stdin/out/err, etc.) That said, I believe that with more advanced native integration we can achieve what we want here while keeping the sys module in Python. Option 2. above is a start in this direction. Once we can assign to Go package variables from Python, we could get rid of the SetStd*() functions in the grumpy package and assign directly to the handles in the Go os package. That's a really nice end state because it means that Go and Python code will share the same std* handles. |
|
PyPy solved it by having non-portable modules "native", something alike: # sys.py (portable)
from _sys import stdout
# _sys.py (non-portable)
from __go__.os import Stdout
from __go__.grumpy import NewFileFromFD
stdout = NewFileFromFD(Stdout)Now going on Option 2) variant...
I do not see a good interface for it to happen. Go variables are not ObjectType instances. The far I can see is Grumpy package variables being assigned from Python, as you said. import __go__.os as go_os
import __go__.grumpy.sys as grumpy_sys
go_os.Stdout = StringIO() # Will never work I guess?
grumpy_sys.Stdout = StringIO() # May work.This way, everything in Grumpy (including Print()) should use grumpy.sys.Stdout, and users would overwrite grumpy.sys.Stdout. Still do not know how to proxy changes to sys.stdout to grumpy.sys.Stdout. Some hack would be needed. |
|
For "some hack", I tried to overwrite setattr of a module in CPython and it let me but have no effect in attribution. Seems as a limitation of CPython that Grumpy could ignore and be more flexible. |
|
Re: go_os.Stdout = StringIO() Yes, this is a tricky case that you'd probably need to jump through hoops to make work with os.Pipe() or something like that. That perhaps creates more problems than it solves. Re: __setattr__ for modules: One approach that should work is to a) define a descriptor (i.e. a property) for std* attributes and b) swap out sys.modules['sys'] with a custom class. Something along these lines: Then basically everything in the sys module would have to live on the _SysModule class. |
|
...And that is the hack we need to grumpy.sys map sys.py !! But after # sys.py
from __go__.grumpy import SysDict, SysModules
__all__ = ['every', 'func', 'implemented', 'in', 'python']
class _SysModule(object):
def __init__(self):
for k in __all__:
SysDict[k] = locals()[k]
def __setattr__(self, name, value):
SysDict[name] = value
def __getattr__(self, name):
return SysDict.get(name)
modules = SysModules
modules['sys'] = _SysModule() |
|
Cool, yeah, something like that would work. I wonder though whether this is getting more hacky than just implementing the whole thing in Go. The Python code in sys.py is pretty far from portable at that point. |
|
I could accept a 10-line hack for every hybrid module in the core... Maybe factored out to the importer if occur too often. Will give a shot and you can decline the PR later if it gets too messy, ok? |
|
Sounds good. Looking forward to seeing how it turns out.
|
|
https://github.com/google/grumpy/blob/master/runtime/object.go#L127 Let me guess: Will open another issue/branch and freeze this one. |
|
You are correct. Those have not yet been implemented. It should be pretty straightforward to add though. |
|
(after send a WIP PR here) |
|
Yes. Will just do in another branch/issue |
|
...and what is my surprise to discover that Now will continue the fix from the google#302 base branch. |
|
Fix proposal: google#304. Needs google#302 (Replaceable sys.stdout via SysmoduleDict) merged first. |
alanjds
added
the
fix available
google#290 opened by @alanjds on 22 Apr 2017
Replacing
sys.stdouthave no effect overprintstatement.This test script outputs
AssertionError: 0 chars printed, instead of 3, after printing 'foo' in the stdout.The text was updated successfully, but these errors were encountered: