I don't know what condition is better. Tests are passed in both cases.

On further thoughts, perhaps the only reliable way is testing the error message. There is possible race condition with testing sys.modules.

I vote for the latter since it's more explicit what you're actually worrying about.

Isn't there a race condition? More precisely, is a race condition here bad?

Is the import lock released at this point? @pitrou might have an opinion on how bad the race condition might be (I personally don't as I've avoided the import lock as much as possible 😄 ).

We're definitely not holding any of the import locks here (not the global one, and not the one for from_name either), so there are two race conditions to consider:

1. Some other thread inserted None into sys.modules while this thread's failed import attempt was running
2. Some other thread successfully imported the module, or inserted a value other than None while this thread's import attempt was running

In neither case do we actually care whether that state change was made before our import attempt, or by a different thread while our import attempt was running, but we do want to treat the cases differently: we want the first one to be considered a failed import, and we want the second one to be considered a successful import.

So I think the sys.modules.get(from_name, _NEEDS_LOADING) is not None check would be the right one to use.

There is yet one possibility. Some other thread started importing the module. It added the module into sys.modules, but still not set the attribute in the parent module.

That's fine, as whether it fails or not will then depend on whether subsequent code uses LOAD_ATTR or IMPORT_FROM, just as it does for circular imports.

You can actually just set __path__ to [] and it will still count as a package.

I vote for the latter since it's more explicit what you're actually worrying about.