Relax the type restriction on reloaded modules

[ericsnowcurrently]

BPO	19468
Nosy	@brettcannon, @ncoghlan, @ericsnowcurrently, @miss-islington, @furkanonder
PRs	bpo-19468: delete unnecessary instance check in importlib.reload() #19424

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-06-05.21:01:25.172>
created_at = <Date 2013-11-01.03:16:53.747>
labels = ['type-feature', 'library']
title = 'Relax the type restriction on reloaded modules'
updated_at = <Date 2020-06-05.21:01:25.171>
user = 'https://github.com/ericsnowcurrently'

bugs.python.org fields:

activity = <Date 2020-06-05.21:01:25.171>
actor = 'brett.cannon'
assignee = 'none'
closed = True
closed_date = <Date 2020-06-05.21:01:25.172>
closer = 'brett.cannon'
components = ['Library (Lib)']
creation = <Date 2013-11-01.03:16:53.747>
creator = 'eric.snow'
dependencies = []
files = []
hgrepos = []
issue_num = 19468
keywords = ['patch']
message_count = 6.0
messages = ['201874', '228273', '228325', '366211', '370781', '370787']
nosy_count = 5.0
nosy_names = ['brett.cannon', 'ncoghlan', 'eric.snow', 'miss-islington', 'furkanonder']
pr_nums = ['19424']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue19468'
versions = ['Python 3.5']

[ericsnowcurrently]

The first thing that importlib.reload() does is to verify that the passed module is an instance of types.ModuleType (Lib/importlib/init.py:107). This check seems unnecessary to me. We really don't have a functional need for the check (that I know of). Furthermore, there has been at least one serious proposal recently that suggested using custom module types.

The only benefit that I can think of to the type check is it makes the failure more clear when someone tries to "reload" an attribute in a module (thinking just the attribute will get reloaded!). However, does that matter all that much now that reload() is not a builtin (ergo less likely to get misused very often)?

I'm not invested in removing these 2 lines (or at least loosening the restriction). I've brought it up simply because it keeps staring me in the face lately. :-) If anyone has any objections, I'll drop it (at least it will be recorded here in the tracker). That said, I'm glad to remove the restriction otherwise.

[BreamoreBoy]

Can we or can't we remove the check as Eric has proposed in msg201874?

[brettcannon]

I think the check can be removed as long as an AttributeError is caught when trying to access module.__name__ and a message mentioning that the user probably meant to pass in a module is used, e.g.:

try:
name = module.__spec__.name
except AttributeError:
try:
name = module.__name__
except AttributeError:
raise TypeError("argument should be a module")

[furkanonder]

PR has been sent.

[miss-islington]

New changeset fef1fae by Furkan Önder in branch 'master':
bpo-19468: delete unnecessary instance check in importlib.reload() (GH-19424)
fef1fae

[brettcannon]

Thanks for the PR, Furkan !