changes with updates printing policy #153
Conversation
Vipul-Cariappa
force-pushed
the
dev/test-update
branch
5 times, most recently
from
06364b5 to
4e5502c
Compare
Vipul-Cariappa
force-pushed
the
dev/test-update
branch
from
4e5502c to
8a34e0f
Compare
| @@ -131,7 +131,7 @@ class Abstract2 { | |||
|
|
|||
| template<class A, class B, class C = A> | |||
| C multiply(A a, B b) { | |||
| return C{a*b}; | |||
| return static_cast<C>(a * b); | |||
There was a problem hiding this comment.
Is that something we could upstream?
There was a problem hiding this comment.
Ok. I will open a PR changing it.
test/test_doc_features.py
Outdated
| @@ -698,9 +696,9 @@ def test09_templated_function(self): | |||
| assert 'multiply' in cppyy.gbl.__dict__ | |||
|
|
|||
| assert mul(1, 2) == 2 | |||
| assert 'multiply<int,int,int>' in cppyy.gbl.__dict__ | |||
| assert 'multiply<int, int, int>' in cppyy.gbl.__dict__ | |||
There was a problem hiding this comment.
Can we make the system accept both space and no space in the comparisons?
| raises(AttributeError, getattr, fragile, "no_such_class") | ||
| # FIXME: Should be fixed with root dictionary | ||
| # look at https://github.com/wlav/cppyy/discussions/306 | ||
| # raises(AttributeError, getattr, fragile, "no_such_class") |
There was a problem hiding this comment.
Our system has no notion of root dictionaries. We will have to fix it in some other way.
There was a problem hiding this comment.
In that case, I guess this should not be part of the test.
| size_t Sizeof() { return s.size() + 1; } | ||
| }; | ||
| } | ||
| """) |
There was a problem hiding this comment.
No. Merged PR upstream: wlav#305.
| """) | ||
|
|
||
| assert not '__abstractmethods__' in dir(cppyy.gbl.docs.ptrDocs) | ||
| assert '__class__' in dir(cppyy.gbl.docs.ptrDocs) |
There was a problem hiding this comment.
Same PR as above.
There was a problem hiding this comment.
Ok, I was confused because this PR apparently does too many things.
Vipul-Cariappa
force-pushed
the
dev/test-update
branch
from
8a34e0f to
ccabc27
Compare
| def test_doc_strings(self): | ||
| import cppyy | ||
| from cppyy.gbl import Concrete | ||
| assert 'void Concrete::array_method(int* ad, int size)' in Concrete.array_method.__doc__ | ||
| assert 'void Concrete::array_method(double* ad, int size)' in Concrete.array_method.__doc__ | ||
| assert 'void Concrete::array_method(int * ad, int size)'.replace(" ", "") in Concrete.array_method.__doc__.replace(" ", "") |
There was a problem hiding this comment.
I think the fix should go in CppInterOp's interfaces that give away strings. That would be a consistent way to make this everywhere.
@wlav, what's your take. How we can make this more resilient to whitespace changes coming from clang's printing?
There was a problem hiding this comment.
I don't care about the spaces per se, although consistency would be nice, this is just a documentation string. But that line should not remove the check for double*. Point being that checking for both is needed to make sure all overloads are represented in the doc string.
No description provided.