print should use __str__ or __repr__ when available - #26
Conversation
Sorry, something went wrong.
Codecov Report
@@ Coverage Diff @@
## master #26 +/- ##
==========================================
- Coverage 64.59% 64.56% -0.04%
==========================================
Files 55 55
Lines 9997 10002 +5
==========================================
Hits 6458 6458
- Misses 3079 3084 +5
Partials 460 460
Continue to review full report at Codecov.
|
Sorry, something went wrong.
ncw
left a comment
There was a problem hiding this comment.
See inline for notes.
Can you do some tests (in python) in builtin/tests/builtin.py too please?
A useful fix - thank you :-)
Sorry, something went wrong.
| for i, v := range args { | ||
| switch sv := v.(type) { | ||
| case py.I__str__: | ||
| v, _ = sv.M__str__() |
There was a problem hiding this comment.
This isn't quite right - this needs to call py.Str which will fall back to py.Repr
If you only look for the M__str__ then you'll miss the user defined methods.
Sorry, something went wrong.
There was a problem hiding this comment.
This works with this test:
class X(object):
def __init__(self, x):
self.x = x
def __str__(self):
return "my name is %s" % self.x
class Y(object):
def __init__(self, x):
self.x = x
def __repr__(self):
return "my nome is %s" % self.x
print(X(42))
print(Y('hello'))
Note that I cannot add any test to builtin/tests/builtin.py because there is no way to check the result of print (until support for redirects is implemented)
Sorry, something went wrong.
There was a problem hiding this comment.
re-implemented with py.Str
Sorry, something went wrong.
|
I have updated with py.Str Can't really add any test builtin/tests/builtin.py right now, since there is no way to test the result of "print" (but I have some work in progress to support print to file) |
Sorry, something went wrong.
|
I see what you mean about the tests. I'll merge this as it is a definite improvement thanks :-) Some tests would be nice at some point! |
Sorry, something went wrong.
|
@ncw In the future, we can add the test which capturing sys.stdout. But not this time :) |
Sorry, something went wrong.
see #24