Do you have tests for the fixes ?

@matthid I left 2 comments in your code, in addition can you please rebase/squash all 4 commits into one logical commit. I agree with@vmuriart that the unit tests would be great to include!

From looking at the commit titles, it looks like 3 commits its about right.Seems to be addressing 3 different issues. in-lieu of doing 3 separatepr, i think 1 with three commits is ok. In case I am wrong, and all three issues are actually the same, then yes should be squashed.

@vmuriart thanks for correcting me :)@matthid you pull request looks good if#194 gets merged!

only tests are missing - not sure if you can get equivalent C# code for your failing F# code?

also need to downgrade your syntax to C# 4.0

@denfromufa Yes I agree with all points. I just wanted to quickly show up the problems. It could take me a while to clean this up as I'm trying to work on something else (I just figured it would be a good idea to let people know there are some fixes available). If someone wants to step in until then feel free to do so. If you encounter problems while reproducing feel free to ask. I'm pretty sure everything can be reproduced with C# as well.

Is there a strong reason to stay on C# 4?

By the way after all these fixes I still see

  clrmodule -> C:\PROJ\googleMusicAllAccess\temp\pythonnet\src\clrmodule\bin\x86\ReleaseWin\clrmodule.dllC:\PROJ\googleMusicAllAccess\temp\pythonnet\packages\UnmanagedExports.1.2.6\tools\RGiesecke.DllExport.targets(42,5): error : Microsoft.Build.Utilities.ToolLocationHelper could not find ildasm.exe. [C:\PROJ\googleMusicAllAccess\temp\pythonnet\src\clrmodule\clrmodule.csproj]

in the build output. So there is still something wrong. As I used a VS developer prompt everything should be available. But I didn't investigate as the build continues and the package is available indist/

@matthid we are tracking this issue with Unmanaged Exports here:

#216

@matthid c# 6 syntax should be downgraded to c# 4 due to .NET 4.0 and Mono compatibility.
This can be re-considered if someone is willing to look into this closer.

Let me first say: I have no problem with downgrading (once I have some time... the project looks a bit dead anyway as there are quite some outstanding PRs). But IMHO those are the wrong reasons. You should ensure mono and net40 compat though build and test.

There is nothing stopping you from compiling c#6 to net40. Only a small subset is not working because of missing .net classes but even those you can get working by defining the missing classes yourself (if really needed). Just google for c#6 and net4 to get sources for this.

The travis build is green, which I would say means it works. Btw later mono versions only run with net45 anyway.

@matthid I read about partial compatibility of C# 6 with .NET 4.0, but did not know that Mono defaults to C# 6. However looks like their main documentation is outdated, which is where I looked first:http://www.mono-project.com/docs/about-mono/languages/csharp/

C# 6 is not ready in Mono compiler according to that page.

WRT pythonnet slow development cycle - please note that it is still more alive development than IronPython and the codebase is older than both IronPython and F#!

@denfromufa Yes Thanks, and the code worked great for me (after the bugfixes at least). I tried IronPython first and couldn't make it do anything :)

Yeah the page is really outdated. Mono moved fast after MS open sourced a lot of things... I think they use Roslyn now and can evolve as fast as Visual Studio...

I cherry picked the original commit from#194 this should make it possible for you to merge both PRs, and it makes it easy to use my branch for other things. Just to be sure you should merge#194 first (but git should be able to handle that just fine).

Additionally I added test for2823941. Now I'm looking into test for35cd5fa. I thinkcc8b796 is quite hard to test...

I'm not using python that much, but as far as I understand it I had to add a module to be able to use relative imports. I hope its fine the way I did it, please review.

Edit:http://stackoverflow.com/questions/16981921/relative-imports-in-python-3

Maybe a stupid question: But where are thePython.Embedding tests run? It seems like they are not run as part of the test suite?

@matthid that is a good point! embedding tests are not run on travis or appveyor. Once I did manage to run them locally and passed, but there is something brittle there with importing file paths:

https://github.com/pythonnet/pythonnet/blob/master/src/embed_tests/pyimport.cs#L27

I opened a separate issue for embedding tests:#224

Ok I managed to reproduce the problem with the "regular" test suite, but I think it might make more sense to add it to the embedding tests (the code might be a bit simpler there).

I think this PR is now ready to review and merge. What about C#6. Should I still change to C#4?

To test that the test cases do indeed reproduce the problems you can use myadd_tests branch. Then cherry-pick the fixes to make the tests work one by one.

@matthid travis CI is failing to build your current pull request:

https://travis-ci.org/pythonnet/pythonnet/jobs/135245011

Yep investigating, locally its working :(

I think this PR is now ready to review and merge. What about C#6. Should I still change to C#4?

No problem, as long as both travis (looks good) and appveyor (have not seen yet) are able to build this.

might be a mono bug, an appveyor run might be cool here to clarify

Yep its a mono bug:

publicclassTestConvert:System.Dynamic.DynamicObject{publicoverrideboolTryConvert(System.Dynamic.ConvertBinderbinder,outobjectresult){result=null;returntrue;}}publicclassTest:System.Dynamic.DynamicObject{publicoverrideboolTryInvokeMember(System.Dynamic.InvokeMemberBinderbinder,object[]args,outobjectresult){result=newTestConvert();returntrue;}publicstaticvoidTestMethod(){dynamict=newTest();stringresult=t.SomeMethod();}}

Test.TestMethod();// Works on .NET, crashes on mono

We could workaround here at pythonnet by checking for PythonsNone and returningnull immediatly in theTryInvokeMember call instead of wrapping in aPyObject instance. Apparently we cannot convert aPyObject to null on mono. Will report this to mono as well but I think we still need a workaround.

https://bugzilla.xamarin.com/show_bug.cgi?id=41509

Done.

You can configure a appveyor account to run off your github account. That
what I did for mine to atleast verify my pr

On Saturday, June 4, 2016, Matthias Dittrichnotifications@github.com
wrote:

Done.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#219 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AMr87ASXXjyppOxhZTa8BDcjpDXKAYrKks5qIZaxgaJpZM4In_8l
.

@vmuriart Yeah, thanks! But I'm testing on windows already locally. Just mentioned it that it would be nice for others to see (because everything was red above)...

I've cherry picked "Alternative Fix for#182" into master. Thanks!

I've cherry picked "Alternative Fix for#182" into master. Thanks!

Ok, but you could have just told me to split the PR or tell me what the problem was with the whole PR (didn't see that there are conflicts until now, because I didn't saw that on the phone).
Fixed the conflicts by rebasing the branch on latest master

apparently in python 2.7.12 some importing issues have been fixed in C-API:

- ``PyImport_Import`` and ``PyImport_ImportModule`` now always do  absolute imports. In earlier versions they might have used relative  imports under some conditions.

https://hg.python.org/cpython/raw-file/v2.7.12/Misc/NEWS

@denfromufa I have no idea what you are trying to tell me or what I should do now with this PR :)

Okay, decision time: Do we allow C#6? Is there any compelling reason not to, as the tests are working?

In my opinion, this PR can be merged.

I thought@tonyroberts cherry-picked this pull request?

C# 6.0 is supported by both CI, so I'm +1 on adopting this!

On Thursday, July 14, 2016, Benedikt Reinartznotifications@github.com
wrote:

Okay, decision time: Do we allow C#6? Is there any compelling reason not
to, as the tests are working?

In my opinion, this PR can be merged.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#219 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AHgZ5YjN_C25nL_rze39HYqLE6NfZrb_ks5qVkfOgaJpZM4In_8l
.

@denfromufa Only parts, not sure whats wrong with the rest. I can only improve this when I know whats missing...

@denfromufa@vmuriart Merge?

@filmor can you give me one day to test this locally? I would like to make
sure that it also works with python 2.7.12 due to the bug fix that I linked
above.

On Fri, Jul 22, 2016 at 3:33 AM, Benedikt Reinartz <notifications@github.com

wrote:

@denfromufahttps://github.com/denfromufa@vmuriart
https://github.com/vmuriart Merge?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#219 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHgZ5QSW5SDEeR1o_E9ce6CsOuzXCBj9ks5qYIB2gaJpZM4In_8l
.

ok, tested locally and it is working! great finding and fix!

C:\Users\denis.akhiyarov>pythonPython 2.7.11 |Anaconda 4.0.0 (32-bit)| (default, Mar  4 2016, 15:18:41) [MSC v.1500 32 bit (Intel)] on win32Type "help", "copyright", "credits" or "license" for more information.Anaconda is brought to you by Continuum Analytics.Please check out: http://continuum.io/thanks and https://anaconda.org>>> import clr>>> import testimport>>> from testimport import relative_importUnhandled Exception: System.ArgumentException: Name must not be empty!   at Python.Runtime.ModuleObject..ctor(String name)   at Python.Runtime.ModuleObject.GetAttribute(String name, Boolean guess)   at Python.Runtime.ImportHook.__import__(IntPtr self, IntPtr args, IntPtr kw)   at Python.Runtime.Runtime.PyObject_Call(IntPtr pointer, IntPtr args, IntPtr kw)   at Python.Runtime.ImportHook.__import__(IntPtr self, IntPtr args, IntPtr kw)C:\Users\denis.akhiyarov>pip uninstall pythonnetUninstalling pythonnet-2.1.0:  c:\python\python27_32b\lib\site-packages\clr.pyd  c:\python\python27_32b\lib\site-packages\python.runtime.dll  c:\python\python27_32b\lib\site-packages\python.runtime.dll.config  c:\python\python27_32b\lib\site-packages\pythonnet-2.1.0.dist-info\description.rst  c:\python\python27_32b\lib\site-packages\pythonnet-2.1.0.dist-info\installer  c:\python\python27_32b\lib\site-packages\pythonnet-2.1.0.dist-info\metadata  c:\python\python27_32b\lib\site-packages\pythonnet-2.1.0.dist-info\metadata.json  c:\python\python27_32b\lib\site-packages\pythonnet-2.1.0.dist-info\record  c:\python\python27_32b\lib\site-packages\pythonnet-2.1.0.dist-info\top_level.txt  c:\python\python27_32b\lib\site-packages\pythonnet-2.1.0.dist-info\wheelProceed (y/n)? y  Successfully uninstalled pythonnet-2.1.0You are using pip version 8.1.1, however version 8.1.2 is available.You should consider upgrading via the 'python -m pip install --upgrade pip' command.C:\Users\denis.akhiyarov>pip install git+https://github.com/matthid/pythonnet.git@myfixesCollecting git+https://github.com/matthid/pythonnet.git@myfixes  Cloning https://github.com/matthid/pythonnet.git (to myfixes) to c:\users\denis~1.akh\appdata\local\temp\pip-inppy5-buildInstalling collected packages: pythonnet  Running setup.py install for pythonnet ... doneSuccessfully installed pythonnet-2.1.0You are using pip version 8.1.1, however version 8.1.2 is available.You should consider upgrading via the 'python -m pip install --upgrade pip' command.C:\Users\denis.akhiyarov>pythonPython 2.7.11 |Anaconda 4.0.0 (32-bit)| (default, Mar  4 2016, 15:18:41) [MSC v.1500 32 bit (Intel)] on win32Type "help", "copyright", "credits" or "license" for more information.Anaconda is brought to you by Continuum Analytics.Please check out: http://continuum.io/thanks and https://anaconda.org>>> import clr>>> import testimport>>> from testimport import relative_importTraceback (most recent call last):  File "<stdin>", line 1, in <module>  File "testimport\relative_import.py", line 4, in <module>    from . import _missing_import  File "testimport\_missing_import.py", line 1, in <module>    import this_package_should_never_exist_everImportError: No module named this_package_should_never_exist_ever>>> exit()C:\Users\denis.akhiyarov>conda update pythonUsing Anaconda Cloud api site https://api.anaconda.orgFetching package metadata .........Solving package specifications: ..........Package plan for installation in environment C:\Python\Python27_32b:The following packages will be downloaded:    package                    |            build    ---------------------------|-----------------    python-2.7.12              |                0        22.4 MB    anaconda-custom            |           py27_0          13 KB    conda-4.1.9                |           py27_0         242 KB    ------------------------------------------------------------                                           Total:        22.6 MBThe following packages will be UPDATED:    anaconda: 4.0.0-np110py27_0 --> custom-py27_0    conda:    4.1.2-py27_0      --> 4.1.9-py27_0    python:   2.7.11-4          --> 2.7.12-0Proceed ([y]/n)? yFetching packages ...python-2.7.12- 100% |###############################| Time: 0:00:14   1.57 MB/sanaconda-custo 100% |###############################| Time: 0:00:00 293.28 kB/sconda-4.1.9-py 100% |###############################| Time: 0:00:00 691.15 kB/sExtracting packages ...[      COMPLETE      ]|##################################################| 100%Unlinking packages ...[      COMPLETE      ]|##################################################| 100%Linking packages ...[      COMPLETE      ]|##################################################| 100%C:\Users\denis.akhiyarov>pythonPython 2.7.11 |Anaconda custom (32-bit)| (default, Mar  4 2016, 15:18:41) [MSC v.1500 32 bit (Intel)] on win32Type "help", "copyright", "credits" or "license" for more information.Anaconda is brought to you by Continuum Analytics.Please check out: http://continuum.io/thanks and https://anaconda.org>>> import clr>>> import testimport>>> from testimport import relative_importTraceback (most recent call last):  File "<stdin>", line 1, in <module>  File "testimport\relative_import.py", line 4, in <module>    from . import _missing_import  File "testimport\_missing_import.py", line 1, in <module>    import this_package_should_never_exist_everImportError: No module named this_package_should_never_exist_ever>>>