Codecov Report

Merging#808 intomaster willdecrease coverage by0.28%.
The diff coverage is55.26%.

@@            Coverage Diff             @@##           master     #808      +/-   ##==========================================- Coverage   76.91%   76.62%   -0.29%==========================================  Files          64       64                Lines        5935     5973      +38       Branches      976      986      +10     ==========================================+ Hits         4565     4577      +12- Misses       1040     1059      +19- Partials      330      337       +7

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last update868cd52...82aea1f. Read thecomment docs.

@koubaa one error here:

=================================== ERRORS ====================================___________________ ERROR collecting src/tests/test_repr.py ___________________ImportError while importing test module 'C:\projects\pythonnet\src\tests\test_repr.py'.Hint: make sure your test modules/packages have valid Python names.Traceback:src\tests\test_repr.py:7: in <module>    from Python.Test import (ReprTest)E   ImportError: cannot import name 'ReprTest' from 'Python.Test' (unknown location)!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!

@denfromufa How do I run the tests locally? I want to make sure they work before pushing. (I just tested manually in the console and tried to follow the pattern)

I figured it out (took longer than I'll admit!)

One test fails because therepr of System.OverflowException went from
OverflowException('Simple message..."
to
<System.OverflowException object at 0xADDRESS>

I'll need to figure out how the old behavior was working to see why its broken now.

@koubaa were you able to run the tests locally? If not, then I can search if we have instructions somewhere.

@denfromufa yes I can run locally now. I have vs2017 installed and ran this to build:

python setup.py install --xplat

then this to test:

python -m pytest

Should this be in the README?

@denfromufa sounds good. I didn't see any embedded test fails in previous CI runs so I'm OK running these partial tests for now.

I tried to find where OverflowExceptionrepr is coming from (on master) and actually no breakpoints in tp_repr defined were hit. So I really don't know (1) how else to look for it and (2) how I could test that its an exception in classbase tp_repr and fallback to python defaults.

@denfromufa I am back from a vacation so I looked at this again. I found thischange which removedtp_repr fromExceptionClassObject. After some trial and error I discovered that__repr__() works on master because of what's done inSetArgsAndCause and the fact that a__repr__() was not defined. Now thatClassBase implementstp_repr, the CPython runtime will not fall back to the args/cause which were set.

My plan is to re-implement repr inExceptionClassObject as was done before that change. Please let me know if you have any concerns with this approach.

This is only partial tests, there are also embedding tests run through nunit. Documentation improvements are welcome, probably this one should go into wiki.

I just edited the wiki with what I know about so far.

python tests now pass in CI. Embedded tests pass locally (on windows) but not in CI, so I'm adding some logging statements to see where.

I'm having a lot of trouble with these CI tests.

• The latest Travisjob is not using my latest changes. Appveyor at least reports the correct SHA1 but have the below errors.
• Appveyor 2.7 x64 xplat hangs for one hour when about to run embedded tests.
• Appveyor 3.4 x86 xplat fails in pip install
• Appveyor 3.4 x64 xplat also fails in pip install
• Appveyor 2.7 x86 seems to have blown up because of the print statements I added. Is there a reason why Console.WriteLine is crashing (i.e. is stdout unavailable?) and if so what's the preferred way to add this sort of debugging logging statements?
• Appveyor 2.7 x64 complains of a test failure but I cannot find much more details.
• Appveyor 3.4 x86 also fails in setuptools
• Appveyor 3.4 x64 also fails in setuptools.

Since all of the appveyor py3.4 are failing in setuptools it seems to me that its fallen out of maintenance. The 1 hour hangs concern me most since everything is queued and it delays other tests.

@filmor we can probably make python 3.4 optional in CI tests as a temporary solution until we drop it?

I'll have a look into this this weekend. Could you please remove the debug output?

done

Seems better but its hard to determine what's wrong with x64 2.7. It says python tests failed but no indication which ones.

From the logs I think that the issue is aimply the casing of theReprTests.cs. In the repository it's written in camel case, while in the csproj file ot'sreprtests.cs. Correct the csproj and it should go through.

@filmor done. Hopefully this works. Do you agree with the newstr behavior()?

Each dot indicates a successful test, so you can just count ;)

I'll have a look into this as soon as I have finally finished the 2.4.0 release.

Codecov Report

Merging#808 intomaster willdecrease coverage by2.65%.
The diff coverage isn/a.

@@            Coverage Diff             @@##           master     #808      +/-   ##==========================================- Coverage   86.71%   84.05%   -2.66%==========================================  Files           1        1                Lines         301      301              ==========================================- Hits          261      253       -8- Misses         40       48       +8

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last update60e6045...264c78b. Read thecomment docs.

Looks like the problems were magically fixed :)

To help you in disagreeing, I propose the another style: We add a new generic abstract class (or interface with default methods when we convert to C#8)PythonWrapper and allow it to be registered for automatic conversion:

using SomeLib;[Python.Conversion]public class SomeClassWrapper : PythonWrapper<SomeClass> {    public string Repr() => $"<SomeClass {this.wrapped.Name}>";}

Now this means that we would need to linkPython.Runtime.dll (or a much smaller DLL that is only used for these markers and the interface) into the wrapper DLL, but this is not an issue in embedding situations and everywhere else we could use the mentioned "smaller", platform-independent DLL.

@filmor not sure I understood your proposal.
Can you give a more complete example? What shouldPythonWrapper<T> do?

Hey, can this go in yet?

It can, thanks a lot for your contribution :)