gh-106915: Add PyImport_ImportOrAddModule() function

[vstinner]

Remove PyImport_AddModuleRef() function.

• Issue: PyImport_AddModuleRef is questionable  #106915

---

📚 Documentation preview 📚: https://cpython-previews--111559.org.readthedocs.build/

[vstinner]

cc @encukou @serhiy-storchaka

[vstinner]

@encukou proposes PyImport_ImportOrAddModule(), nobody proposes better name. I abandon my PyImport_CreateModule() name: I renamed the function to PyImport_ImportOrAddModule().

[vstinner]

Oh, if PyImport_AddModuleRef() is removed, I should update pythoncapi-compat as well: https://pythoncapi-compat.readthedocs.io/en/latest/api.html#c.PyImport_AddModuleRef

[encukou]

API-wise, this looks good to me, thank you! We should of course wait if the C-API WG is approved.

In the docs, I'd mention creating a new (empty) module more prominently, to make extra sure people don't assume the module is imported if missing.

[vstinner]

I applied your suggestions manually to also update the doc in Include/import.h at the same time.

[vstinner]

We should of course wait if the C-API WG is approved.

The working group doesn't exist yet.

Maybe @gvanrossum @zooba and @iritkatriel want to review this PR?

[vstinner]

@encukou: Would you mind to review the updated PR?

[encukou]

I'll get to it next week.

[encukou]

(Got to it sooner!)

+1 from me on the functionality. Thank you!

It looks OK API-wise, too. I recommend checking with the other CAPI-workgroup-to-be members if it fits the direction they want.

One thing to note is the allocation of the “lesser” and “greater” result (0 or 1) for this kind of “setdefault”-style API: should 1 mean “created/assigned” like here, or “found” like in “getitem”-style API?
As far as I'm concerned, it could go either way, but some people might have a strong opinion. And this could become a precedent.

[vstinner]

One thing to note is the allocation of the “lesser” and “greater” result (0 or 1) for this kind of “setdefault”-style API: should 1 mean “created/assigned” like here, or “found” like in “getitem”-style API?

Aha. The function is called import or add: add comes second, so I suppose that it should return success (1) if import is successful. Also, I think that returning 1 if the module exists in sys.modules which be consistent with PyDict_GetItemRef() API.

I modified the PR to return 1 if the module is found in sys.modules, or 0 if the module was created (and then added to sys.modules).

[vstinner]

I modified the PR to return 1 if the module is found in sys.modules, or 0 if the module was created (and then added to sys.modules).

@encukou: Would you mind to review the updated PR?

[encukou]

I like this better, thanks!
Again, it would be nice if the other candidates for C-API WG confirmed that this matches the direction they want to take.

[zooba]

I think we've still got to thoroughly discuss the return value convention, and personally in this case I'd prefer to see PyObject *PyImport_ImportOrAddModule(const char *name, int *was_created) where the second parameter can be NULL if you don't care.

Right now it isn't immediately obvious whether the existing value of the module argument is what is going to be added, or whether a new one will be created (ImportOrCreateModule might be enough to clarify, but as it's a "get the object" type API, and the object knows how to alloc/dealloc itself, I'd prefer the return value to be the object rather than the disposition of the object).

If there's a need to hurry this API in, then I won't block it, but I would ask that it not be used as precedent when we come to deciding the guideline. If we decide to go the other way, it can just be an exception/wart, but we shouldn't rush an API and then use that to determine the guideline.

[vstinner]

@zooba:

personally in this case I'd prefer to see PyObject *PyImport_ImportOrAddModule(const char *name, int *was_created) where the second parameter can be NULL if you don't care.

Well, I added PyImport_AddModuleRef() to Python 3.13 in replacement of PyImport_AddModuleObject() to avoid borrowed references (the implementation uses a strange hack using a weakref). Then @encukou created issue gh-106915 since he considers that we should provide the information to the caller if the module was get from sys.modules or if a new empty main was created. Currently, the information is not provided, and I don't recall previous request to get it.

@encukou: So, which API do you prefer?

• Keep PyObject* PyImport_AddModuleRef(const char *name) as it is?
• int PyImport_ImportOrAddModule(const char *name, PyObject **module) (module cannot be NULL)
• PyObject *PyImport_ImportOrAddModule(const char *name, int *was_created) where was_created can be NULL

[encukou]

I think we've still got to thoroughly discuss the return value convention

Please add your proposal/ideas there. New API is being added without waiting for that discussion...

Well, I added PyImport_AddModuleRef() to Python 3.13 in replacement of PyImport_AddModuleObject() to avoid borrowed references (the implementation uses a strange hack using a weakref). Then @encukou created issue #106915 since he considers that we should provide the information to the caller if the module was get from sys.modules or if a new empty main was created. Currently, the information is not provided, and I don't recall previous request to get it.

As always with new API, I'd prefer waiting until the WG can discuss it.
I think there are multiple issues with PyImport_AddModuleRef. I'd prefer to get a proper design before adding API, but I feel rushed in this case -- if we don't solve it for 3.13, we'll be stuck with PyImport_AddModuleRef.
Also, I don't think this function is particularly important, and it would be fine to keep things as they were in 3.12. The borrowed reference is just one of the warts.

@encukou: So, which API do you prefer?

What I prefer now, in the middle of the discussions?

1. For importing stdlib or other stuff import can handle, PyImport_ImportModule, which exists already
2. For a new empty module, something like PyObject *PyImport_AddNew(char *name)
3. For a new “namespace” module or one shared with other extensions: the thread-safe getdefault pattern I proposed above.

For 3., the PyImport_ImportOrCreate is OK, except for that thread-safety issue when users need to initialize the module.
For 2., PyImport_ImportOrCreate is also OK, provided that the user handles the case where the module already existed -- then they'd want to do one of two things:

• raise an error. Perhaps it would, in general, be good to provide API that raises errors, in order to have consistent exceptions.
• skip any initialization & reuse the module. But in case they needed initialization, there's a race condition again.

[vstinner]

This PR implements what @encukou proposed there: #106915 (comment) But apparently, @encukou now dislikes the API.

I don't have the bandwidth to dig into the issue. @encukou: if you're motivated, please go ahead. For now, I prefer to close this PR.