Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise ValueError when __loader__ not defined for importlib.find_loader() #61301

Closed
brettcannon opened this issue Feb 1, 2013 · 5 comments
Closed
Assignees
Labels
easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@brettcannon
Copy link
Member

BPO 17099
Nosy @warsaw, @brettcannon, @theller, @ncoghlan, @asvetlov, @ericsnowcurrently

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 = 'https://github.com/brettcannon'
closed_at = <Date 2013-03-13.18:11:19.795>
created_at = <Date 2013-02-01.13:44:37.446>
labels = ['easy', 'type-bug', 'library']
title = 'Raise ValueError when __loader__ not defined for importlib.find_loader()'
updated_at = <Date 2013-03-13.18:11:19.794>
user = 'https://github.com/brettcannon'

bugs.python.org fields:

activity = <Date 2013-03-13.18:11:19.794>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2013-03-13.18:11:19.795>
closer = 'brett.cannon'
components = ['Library (Lib)']
creation = <Date 2013-02-01.13:44:37.446>
creator = 'brett.cannon'
dependencies = []
files = []
hgrepos = []
issue_num = 17099
keywords = ['easy']
message_count = 5.0
messages = ['181079', '181084', '181356', '184097', '184098']
nosy_count = 9.0
nosy_names = ['barry', 'brett.cannon', 'theller', 'ncoghlan', 'asvetlov', 'python-dev', 'eric.snow', 'gkcn', 'Ankur.Ankan']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'test needed'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue17099'
versions = ['Python 3.3', 'Python 3.4']

@brettcannon
Copy link
Member Author

If __loader__ is None then ValueError is raised, but if it is not defined then AttributeError is raised instead. Probably should harmonize around ValueError even in the missing attribute case since __loader__ = None is equivalent to the attribute not existing.

I'm on the fence about considering this a bug, though, since the docs say if __loader__ == None then ValueError but does not directly mention what happens if the attribute is missing. Since anyone who has written code for this probably is catching both attributes (if at all since all but three modules coming from Python will have __loader__ defined ATM), it should be fine, but it is still a change in API/semantics that doesn't contradict the docs.

@brettcannon brettcannon added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Feb 1, 2013
@brettcannon
Copy link
Member Author

Should mention that this is probably no harder than changing a key getattr() call to None (as pointed out by Nick).

@brettcannon brettcannon self-assigned this Feb 3, 2013
@ericsnowcurrently
Copy link
Member

My vote is for making this a ValueError in both cases (and amending the doc appropriately as well). The error amounts to the same thing: the module did not have loader (implicitly or explicitly). If someone wants to distinguish between the two they can explicitly check __loader__ on the module.

Incidently, _find_and_load_unlocked() has similar code to find_loader() (Lib/importlib/_bootstrap:1523) and raises ImportError instead of ValueError. That's actually fine since it's a different situation. However, _find_module() does not handle when __loader__ does not exist, so you would get neither ValueError nor ImportError. I expect we'd want it or _find_and_load_unlocked() to convert the AttributeError into ImportError to be consistent both with the fix for this issue and with how we handle the __loader__ == None case there.

---

For reference, here is the original python-dev thread:

http://mail.python.org/pipermail/python-dev/2013-January/123777.html

The reference to ValueError is in the importlib docs:

http://docs.python.org/dev/library/importlib.html#importlib.find_loader

@python-dev
Copy link
Mannequin

python-dev mannequin commented Mar 13, 2013

New changeset cee04627bdd0 by Brett Cannon in branch 'default':
Issue bpo-17099: Have importlib.find_loader() raise ValueError when
http://hg.python.org/cpython/rev/cee04627bdd0

@brettcannon
Copy link
Member Author

I decided not to backport since it shifts what exception is raised.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants