Okay, I didn't read closely enough. :)  It may be worth updating the title.

FWIW, the name "module_from_spec" confused me at first because my brain interpreted that as "load_from_spec".  Keeping the name and purpose more focused might be helpful.  I have comments below to that effect.

If your proposed change still makes sense, could we keep it simpler for now?  Something like this:

# in importlib.util
def module_from_spec(spec, module=None):
    """..."""
    methods = _bootstrap._SpecMethods(spec)
    if module is None:
        return methods.create()
    else:
        methods.init_module_attrs(methods)
        return module

Keeping the 2 methods on _SpecMethods is helpful for the PEP 406 (ImportEngine) superseder that I still want to get back to for 3.5. :)

----------------------------------

> This serves two purposes. One is that it abstracts the
> loader.create_module() dance out so that's no longer a worry.

I'm not sure what dance you mean here and what the worry is.

> But more crucially it also means that if you have the function
> create the module for you then it will be returned with all of
> its attributes set without having to worry about forgetting that
> step.

So we would discourage calling ModuleType directly and encourage the use of a function in importlib.util that is equivalent to _SpecMethods.create().  That sounds good to me.  The use case for creating module objects directly is a pretty advanced one, but having a public API for that would still be good.

From this point of view I'd expect it to just look like this:

def new_module(spec):
    return _SpecMethods(spec).create()

or given what I expect is the common use case currently:

def new_module(name, loader):
    spec = spec_from_loader(name, loader)
    return _SpecMethods(spec).create()

or together:

def new_module(spec_or_name, /, loader=None):
    if isinstance(spec_or_name, str):
        name = spec_or_name
        if loader is None:
            raise TypeError('missing loader')
        spec = spec_from_loader(name, loader)
    else:
        if loader is not None:
            raise TypeError('got unexpected keyword argument "loader"')
        spec = spec_or_name
    return _SpecMethods(spec).create()

To kill 2 birds with 1 stone, you could even make that the new signature of ModuleType(), which would just do the equivalent under the hood.  That way people keep using the same API that they already are (no need to communicate a new one to them), but they still get the appropriate attributes set properly.

> The module argument is just for convenience in those
> instances where you truly only want to override the module
> creation dance for some reason and really just want the
> attribute setting bit.

Perhaps it would be better to have a separate function for that (equivalent to just _SpecMethods.init_module_attrs()).  However, isn't that an even more uncommon use case outside of the import system itself?

> About the only other thing I can think of that people might
> still want is something like `importlib.util.load(spec)`

I agree it's somewhat orthogonal.  I'll address comments to issue #21235.