[WIP] Add ability to create module from C# #1447

slide · 2021-04-21T23:38:24Z

What does this implement/fix? Explain your changes.

This allows doing something like this, which I haven't found any other way to do:

var mod = PythonEngine.AddModule("mymod");
var myobj = new object();
mod.SetAttr("myprop".ToPython(), myobj.ToPython());

import mymod
print(mymod.myprop)

This is very useful for providing objects to an embedded Python engine.

Does this close any currently open issues?

I could not find any specific issues that would be related.

Any other comments?

This is an initial PR to get feedback if this would be allowed, I have a build of the library locally with this change and use this in my code, but would like to have it upstream if possible. I will fill out the remaining details and add tests if folks think this would be ok.

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 to AUTHORS
Updated the CHANGELOG

lostmsu · 2021-04-22T00:57:49Z

We now have https://github.com/pythonnet/pythonnet/blob/master/src/runtime/pymodule.cs
I think it would make sense to give it a static factory method Create instead of placing code into PythonEngine.

Also, you should probably use

pythonnet/src/runtime/runtime.cs

Line 1895 in 16f04e9

internal static NewReference PyModule_New(string name)

slide · 2021-04-22T03:19:55Z

Cool, I didn't know about pymodule.cs, I'll definitely migrate there and use the PyModule_New

…addmodule

lostmsu · 2021-04-27T07:09:24Z

src/embed_tests/TestPyModule.cs

+
+namespace Python.EmbeddingTest
+{
+    class TestPyModule


I think test class must be public.

lostmsu · 2021-04-27T07:21:26Z

src/runtime/pymodule.cs

@@ -37,5 +40,40 @@ public static PyModule FromString(string name, string code)
            PythonException.ThrowIfIsNull(m);
            return new PyModule(ref m);
        }
+
+        public static PyModule Create(string name, string filename = "none")


This method violates single responsibility principle.

First, Create should never return an existing module. Instead, the caller should be able to check if module already exists by name by calling a separate function.

Second, I am uncertain about setting up __builtins__ by default. It would be great to know why Python does not do it before discarding that potentially good rationale. Users can always manually do newModule.Dict.Set("__builtins__", GetBuiltIns()). If this is a common scenario in your product, just make a factory method combining the two.

Third, I don't think adding new module to sys.modules right away is a good idea. Especially, overwriting the existing value. This might cause hard to debug surprise issues.

Second, I am uncertain about setting up __builtins__ by default. It would be great to know why Python does not do it before discarding that potentially good rationale. Users can always manually do newModule.Dict.Set("__builtins__", GetBuiltIns()). If this is a common scenario in your product, just make a factory method combining the two.

I didn't see a public method that would allow access to the __builtins__. There is an internal method, on Runtime, but not a public method that could be used outside.

lostmsu · 2021-04-27T07:22:04Z

src/runtime/pymodule.cs

+            PythonException.ThrowIfIsNull(__builtins__);
+            int rc = Runtime.PyDict_SetItemString(globals, "__builtins__", __builtins__);
+            PythonException.ThrowIfIsNotZero(rc);
+            rc = Runtime.PyDict_SetItemString(globals, "__file__", new BorrowedReference(filename.ToPython().Handle));


The default value for filename should be null and when it is null __file__ should simply not be set.

LimpingNinja · 2021-07-24T19:34:50Z

This is a good add feature, is anyone working on it? I am migrating from C where I create a handful of accessible modules from C using Py_InitModule3() and passing in the methods (C function pointers) and vars. Is there an alternative method here or is this something that would pend this change being actioned?

slide · 2021-07-25T03:53:43Z

I need to get back to it and make the changes that were requested. I just have a local compiled version with my changes that I have been using, so I haven't had a huge push to get me back to fixing this up. I'll try and work on it soon.

slide · 2021-07-27T23:57:09Z

@lostmsu I think I addressed the feedback you provided, please let me know if there are more changes you would like me to make. If not, I will make this a normal PR and remove the WIP.

src/runtime/pymodule.cs

slide · 2021-07-29T02:07:29Z

Feedback hopefully addressed. Thanks for being patient. I still don't know the codebase very well.

Add ability to create module from C#

ea87103

slide added 2 commits April 26, 2021 11:34

Merge branch 'master' of https://github.com/pythonnet/pythonnet into …

807252a

…addmodule

Move to PyModule and add test

bf679dc

lostmsu requested changes Apr 27, 2021

View reviewed changes

slide added 2 commits July 27, 2021 15:18

Merge remote-tracking branch 'origin/master' into addmodule

a5218ed

Changes based on feedback

f872b3d

lostmsu requested changes Jul 28, 2021

View reviewed changes

src/runtime/pymodule.cs Outdated Show resolved Hide resolved

src/runtime/pymodule.cs Outdated Show resolved Hide resolved

src/runtime/pymodule.cs Outdated Show resolved Hide resolved

src/runtime/pymodule.cs Outdated Show resolved Hide resolved

src/runtime/pymodule.cs Show resolved Hide resolved

lostmsu reviewed Jul 28, 2021

View reviewed changes

src/runtime/pymodule.cs Show resolved Hide resolved

slide added 2 commits July 28, 2021 14:26

Fix some of the feedback

d4a32ed

More feedback fixup

ab0cedb

lostmsu approved these changes Jul 29, 2021

View reviewed changes

lostmsu merged commit 9555248 into pythonnet:master Jul 29, 2021

filmor mentioned this pull request Aug 1, 2021

how to execute within Entrance "if __name__ == "__main__":" #1495

Closed

slide deleted the addmodule branch September 29, 2022 15:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[WIP] Add ability to create module from C# #1447

[WIP] Add ability to create module from C# #1447

Uh oh!

slide commented Apr 21, 2021

Uh oh!

lostmsu commented Apr 22, 2021

Uh oh!

slide commented Apr 22, 2021

Uh oh!

lostmsu Apr 27, 2021

Uh oh!

lostmsu Apr 27, 2021 •

edited

Loading

Uh oh!

slide Jul 27, 2021 •

edited

Loading

Uh oh!

lostmsu Apr 27, 2021

Uh oh!

LimpingNinja commented Jul 24, 2021

Uh oh!

slide commented Jul 25, 2021

Uh oh!

slide commented Jul 27, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slide commented Jul 29, 2021

Uh oh!

Uh oh!

[WIP] Add ability to create module from C# #1447

[WIP] Add ability to create module from C# #1447

Uh oh!

Conversation

slide commented Apr 21, 2021

What does this implement/fix? Explain your changes.

Does this close any currently open issues?

Any other comments?

Checklist

Uh oh!

lostmsu commented Apr 22, 2021

Uh oh!

slide commented Apr 22, 2021

Uh oh!

lostmsu Apr 27, 2021

Choose a reason for hiding this comment

Uh oh!

lostmsu Apr 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slide Jul 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lostmsu Apr 27, 2021

Choose a reason for hiding this comment

Uh oh!

LimpingNinja commented Jul 24, 2021

Uh oh!

slide commented Jul 25, 2021

Uh oh!

slide commented Jul 27, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slide commented Jul 29, 2021

Uh oh!

Uh oh!

lostmsu Apr 27, 2021 •

edited

Loading

slide Jul 27, 2021 •

edited

Loading