Author eric.snow
Recipients Arfrever, aronacher, brett.cannon, eric.snow, ncoghlan, rhettinger
Date 2014-05-26.18:29:14
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <CALFfu7D3gVc_Sk4NTertg+tYQEhi3a5JD9RkXG8_8oAP-Vn9uA@mail.gmail.com>
In-reply-to <1401115022.93.0.790521202511.issue20383@psf.upfronthosting.co.za>
Content
tl;dr I'm okay with pulling the functions out of _SpecMethods (and
dropping the class) but I'd prefer the distinct functions continue to
exist as they are.  Also, I still think updating the ModuleType
signature is the way to go (given current use cases). :)

> First, about breaking up _SpecMethods: that was entirely on purpose. =) ... IOW _SpecMethods feels like OOP just 'cause and not for any design reasons; it's been making my eye twitch when I look at that class.

Keep in mind that _SpecMethods is basically a proxy for ModuleSpec
that has the methods that ModuleSpec had originally.  That was the
simplest way to not expose the methods on every module.__spec__.  If
we didn't care about exposing more API on __spec__, we'd have it all
on ModuleSpec.  I still wonder if we could have used subclassing
instead of proxying (obviating the pain points we've run into) and
used a SimpleModuleSpec for __spec__ and ModuleSpec for all other APIs
(or ModuleSpec/FullModuleSpec):

  SimpleModuleSpec  # What currently is ModuleSpec.
  ModuleSpec(SimpleModuleSpec)  # Basically this is _SpecMethods.

Regardless,  _SpecMethods made sense at the time and I still find it
helpful to have the functions grouped into a distinct namespace.  That
said, I agree that it's a pain. :)  I'm okay with ditching the class,
but would rather we kept the distinct functions that exist now.

> Second, the dance that an advanced user has to worry about is "does create_module exist, and if so did it not return None and if any of that is not true then return what types.ModuleType would give you" is not exactly one line of code ATM. There is no reason to not abstract that out to do the right thing in the face of a loader.

That's what _SpecMethods.create() already does for you.

> Third, yes this would be to encourage people not to directly call types.ModuleType but call the utility function instead.

I'm totally on board. :)

> Fourth, I purposefully didn't bifurcate the code of types.ModuleType based on the type of what the first argument was. At best you could change it to take an optional spec as a second argument and use that, but if you did that and the name doesn't match the spec then what?

I see your point.  I just see the trade-offs a little differently. :)
This is all about finding the best way of eliminating the problem of
people not setting the module attributes properly (and making it
easier to do so).  From my perspective:

* Currently every module must have a spec (and consequently a loader).
* Updating the ModuleType signature will encourage setting the spec,
etc. better than a separate factory would.

If every module must have a spec, then I'd expect that to be part of
the ModuleType signature.  I can see the use case (though uncommon)
for directly creating a module object.  Is there a use case for
creating a module without a spec?  Either way, if it's important
enough to ensure that people set the module attrs properly, then we
should consider having direct instantiation of ModuleType() issue a
warning when created without setting the spec, etc.

Regarding the second point, with a separate factory function, people
will have to learn about the function and then switch to using it
before they get the benefit.

Backward compatibility for an updated signature shouldn't be too hard:

  currently: ModuleType(name, doc=None)
  updated: ModuleType(name_or_spec, doc=None, *, loader=None)
  actual: ModuleType(name_or_spec=None, /, name=None, doc=None, *, loader=None)

> I'm not going to promote passing in a loader because spec_from_loader() cannot necessarily glean all of the same information a hand-crafted spec could if the loader doesn't have every possible introspection method defined (I'm calling "explicit is better than implicit" to back that up).

Regardless of new signature or new factory, I still think the
signature should allow for passing in a spec or passing in a
name/loader pair.  Passing name/loader instead of spec is a
convenience for what I anticipate is the most common use case now:
given a loader, create a new module.  For comparison:

  mod = new_module(name, loader=loader)

vs.

  spec = spec_from_loader(name, loader=loader)
  mod = new_module(spec)

I'll argue that we can accommodate the common case and if that doesn't
work, they can still create their own spec.  If this were new
functionality then I'd agree with you.

> I also don't think any type should depend on importlib having been previously loaded to properly function if it isn't strictly required, so the code would have to be written entirely in C which I just don't want to write. =)

If it's in _bootstrap then it's already available.  No C required. :)

> Since it's going beyond simply constructing a new module but also initializing it I think it's fine to keeping it in importlib.util which also makes it all more discoverable for people than having to realize that types.ModuleType is the place to go to create a module and get its attributes initialized.

It seems to me that people haven't been thinking about initializing a
module's attributes.  This is popping up now because people are
starting to take their advanced importing use cases to Python 3 (which
I take as a very good sign!).  So ModuleType seems like the right
place for me. :)
History
Date User Action Args
2014-05-26 18:29:16eric.snowsetrecipients: + eric.snow, brett.cannon, rhettinger, ncoghlan, aronacher, Arfrever
2014-05-26 18:29:15eric.snowlinkissue20383 messages
2014-05-26 18:29:14eric.snowcreate