Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

First chunk of modernisation changes#1109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
filmor wants to merge13 commits intopythonnet:masterfromfilmor:modernisation-light

Conversation

filmor
Copy link
Member

  • Move projects around and add clean project/solution files
  • Drop monoclr and clrmodule
  • Add stub clr module
  • Drop nuget config
  • Add test helper project
  • Drop PYTHON3 flag (default now)
  • Remove unneeded additional interop files
  • Make NETSTANDARD the default
    Drops also
    • Custom XDecref/XIncref (should be readded)
    • Remote object handling for .NET Framework
  • Remove Runtime._UCS dependency from CustomMarshaler
  • Stop using Runtime.IsPython2/3

What does this implement/fix? Explain your changes.

...

Does this close any currently open issues?

...

Any other comments?

...

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself toAUTHORS
  • Updated theCHANGELOG

- Move projects around and add clean project/solution files- Drop monoclr and clrmodule- Add stub clr module- Drop nuget config- Add test helper project- Drop PYTHON3 flag (default now)- Remove unneeded additional interop files- Make NETSTANDARD the default    Drops also    - Custom XDecref/XIncref (should be readded)    - Remote object handling for .NET Framework- Remove Runtime._UCS dependency from CustomMarshaler- Stop using Runtime.IsPython2/3
@dnfclas
Copy link

dnfclas commentedApr 10, 2020
edited
Loading

CLA assistant check
All CLA requirements met.

- Swapped UCS2 in one place- Explicitly acquire GILState in InternalInitialize- Fix a few problems with short-ints by using larger numbers  (`PyLongType`) or dropping the respective return (`AtExit`)
@filmor
Copy link
MemberAuthor

@koubaa@lostmsu I've gotten quite a bit further. The current state of this PR implements embedding in .NET Core and Mono on Linux (haven't tested yet on Windows, will not be able to test on OS X), I tested with the following script:

importos#os.environ["MONO_LOG_LEVEL"] = "debug"#os.environ["MONO_LOG_MASK"] = "cfg,dll"#os.environ["COREHOST_DEBUG"] = "1"#os.environ["COREHOST_TRACE"] = "1"#os.environ["COREHOST_TRACE_VERBOSITY"] = "4"importpythonnet,clr_loaderimportsysifsys.argv[1]=="mono":mono=clr_loader.get_mono()rt=monoelifsys.argv[1]=="core":core=clr_loader.get_coreclr("/home/benedikt/git/clr-loader/example/out/example.runtimeconfig.json")rt=corepythonnet.set_runtime(rt)pythonnet.load()print("Loaded pythonnet")importclrfromSystemimportConsoleConsole.WriteLine("Success")

Still crashes when I try to do this from IPython, but we're getting somewhere :)

Next steps are:

  • Make aclr.py that handles these initial load steps like they worked before and make the test-suite work
  • See how this works for the other direction (embedding Python in C#)
  • Test on Windows in all variants
  • Allow for a simpler way to configure .NET Core (currently requires aruntimeconfig.json)

@koubaa
Copy link
Contributor

koubaa commentedApr 12, 2020
edited
Loading

@filmor this PR seems to accomplish a lot, I wish it could be staged so it would be easier to follow. For instance, does this also fix 857 and drop support for python2?

Also the file renames I think should be done in a separate PR to better understand the scope. If you like I can take care of adding some of these easier stages.

sidenote I totally agree with dropping remoting support. If someone wants that they should maintain a fork.

lostmsu reacted with thumbs up emoji

@filmor
Copy link
MemberAuthor

This doesn't drop Python 2 support, yet, it merely defaults to Python 3 unlessPYTHON2 is defined. I don't think moving around the files and sticking to the old csproj file format makes sense. Regarding stages, be my guest, I just don't want to redo all parts of this multiple times. Moving to the new project format only really works while simultaneously dropping the old Mono CLR loader, which then requires the new one to be in place for tests to be able to run.

What I want to accomplish in the end here is:

  1. New project format
  2. Use ofclr-loader
  3. .NET Standard only
  4. Reasonable structure for theinterop*.cs files

@koubaa
Copy link
Contributor

@filmor I'll push up a PR with some things staged. I think the smaller and more incremental we can make this branch the easier it'll be to land (not to mention to allow users to bisect if it breaks anyone!)

My first PR will be:

  • default to python 3
  • drop custom xincref/xdecref & remoting
  • Stop using Runtime.IsPython2/3

Which variants do we need to test on windows? I can also work on that + embedding.

@filmor
Copy link
MemberAuthor

@koubaa Have a look at the other PR (#1015), it has much smaller grained commits. The reason I went for this larger variant is that I did some steps in the "wrong" order (first moving the files around, then modifying them).

The parts you are suggesting may safely go against master, all steps that change the project structure can't. Please ensure that they are compatible with this branch, though.

I don't understand what you mean by "variants".

@koubaa
Copy link
Contributor

@filmor I am referring to a task in your comment 3 days ago: "Test on Windows in all variants"

@filmor
Copy link
MemberAuthor

On Windows we can use all .NET runtimes, i.e. Framework, Core and Mono, so all of those should be tested.

@koubaa
Copy link
Contributor

@filmor understood. I think I can manage that. In the meantime I pushed the first stage here:#1117

try
{
gilState = Runtime.PyGILState_Ensure();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Does this have to be insidetry block?

<Project Sdk="Microsoft.NET.Test.Sdk">

<PropertyGroup>
<TargetFrameworks>netcoreapp3.0;net472</TargetFrameworks>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

netcoreapp3.0 will soon be unsupported. We should use netcoreapp3.1 which is LTS

</ItemGroup>

<ItemGroup Condition="$(TargetFramework) == 'net472'">
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0-preview.2">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I think non-preview version of this package was released.

@filmorfilmor closed thisJan 29, 2021
@filmorfilmor deleted the modernisation-light branchJanuary 29, 2021 06:57
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@lostmsulostmsulostmsu left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@filmor@dnfclas@koubaa@lostmsu

[8]ページ先頭

©2009-2025 Movatter.jp