Issue37444
Created on 2019-06-28 19:22 by brett.cannon, last changed 2020-01-23 17:51 by brett.cannon. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 14869 | merged | nsiregar, 2019-07-20 15:36 | |
| Messages (13) | |||
|---|---|---|---|
| msg346855 - (view) | Author: Brett Cannon (brett.cannon) * | Date: 2019-06-28 19:22 | |
builtins.__import__() raises ValueError (as it did in Python 2.7) while importlib.__import__() raises ImportError (which makes more sense to me). Found by Ben Lewis. |
|||
| msg346858 - (view) | Author: Brett Cannon (brett.cannon) * | Date: 2019-06-28 19:26 | |
Importllib code raising the exception is https://github.com/python/cpython/blob/f9f8e3ce709ceb15c8db8c8dde940daf1febf13d/Lib/importlib/_bootstrap.py#L874-L876 . |
|||
| msg346859 - (view) | Author: Brett Cannon (brett.cannon) * | Date: 2019-06-28 19:26 | |
I think this should be an ImportError and so that means builtins.__import__() is wrong. Anyone want to argue for the other side? |
|||
| msg346860 - (view) | Author: Brett Cannon (brett.cannon) * | Date: 2019-06-28 19:28 | |
import.c code raising the exception: https://github.com/python/cpython/blob/f9f8e3ce709ceb15c8db8c8dde940daf1febf13d/Python/import.c#L1675-L1677 |
|||
| msg347021 - (view) | Author: Nick Coghlan (ncoghlan) * | Date: 2019-07-01 14:00 | |
ImportError sounds right to me. We already raise that just above this for the "no dots at all" case, so also raising it for the "not enough dots" case makes more sense than leaving them different. |
|||
| msg348215 - (view) | Author: Raymond Hettinger (rhettinger) * | Date: 2019-07-20 16:23 | |
> I think this should be an ImportError I also think ImportError is what people would expect when an import fails. |
|||
| msg348943 - (view) | Author: miss-islington (miss-islington) | Date: 2019-08-03 05:46 | |
New changeset c5fa44944ee0a31a12b9a70776c7cb56c4dc39a2 by Miss Islington (bot) (Ngalim Siregar) in branch 'master': bpo-37444: Update differing exception between builtins and importlib (GH-14869) https://github.com/python/cpython/commit/c5fa44944ee0a31a12b9a70776c7cb56c4dc39a2 |
|||
| msg360394 - (view) | Author: Miro Hrončok (hroncok) * | Date: 2020-01-21 12:01 | |
While raising ImportError certainly makes much more sense in this case, this change is backwards incompatible and there was no DeprecationWarning. I don't consider that fair. As a certain compromise, we could maybe raise a custom ImportError+ValueError offspring (e.g. BeyondToplevelImportError)? If feeling fancy, we could rise a DeprecationWarning when issublcass(BeyondToplevelImportError, ValueError) is used? |
|||
| msg360433 - (view) | Author: Brett Cannon (brett.cannon) * | Date: 2020-01-21 21:12 | |
There's no deprecation warning because how do you warn about that? You raise DeprecationWarning about an exception you have yet to raise? And I personally think calling this unfair is a bit harsh. |
|||
| msg360434 - (view) | Author: Miro Hrončok (hroncok) * | Date: 2020-01-21 21:17 | |
I know that raising a DeprecationWarning here is most likely not possible or too black-magical to do. My intention was not to be harsh, sorry about that. I just wanted to point out that a backwards incompatible behavior like this without a (possible) deprecation warning might not counterweigh the gain of raising the logical exception type. As a compromise, I proposed to raise a hybrid that inherits from both. |
|||
| msg360496 - (view) | Author: Brett Cannon (brett.cannon) * | Date: 2020-01-22 18:36 | |
The problem with the hybrid exception is that importlib would then still raise a different exception from import itself, so the discrepancy would persist. And if you updated import you could then break people in another way where they were catching ValueError and ImportError separately for different cases and now they would potentially catch the wrong exception for that ValueError case. Basically changing what exceptions get raised sucks as someone is almost inevitably broken. But since import and importlib strive to mirror each other as much as possible and be drop-in replacements, the consistency for the next 30 years of Python's life is more important I think than the case of: 1. Someone using importlib.import_module() 2. Who is specifically worried about relative imports going too far and no other import-related import and thus only catching ValueError and not ImportError 3. And are supporting older versions of Python 4. And will have a difficult time updating their `except` clause to now also capture ImportError |
|||
| msg360523 - (view) | Author: Miro Hrončok (hroncok) * | Date: 2020-01-22 23:53 | |
For the record, looking at https://docs.python.org/3.9/whatsnew/3.9.html I had an impression that this also affects regular imports, as described in https://bugzilla.redhat.com/show_bug.cgi?id=1791769#c2 -- if that's not that case, I'm sorry. Anyway, I get your point of "next 30 years of Python". |
|||
| msg360570 - (view) | Author: Brett Cannon (brett.cannon) * | Date: 2020-01-23 17:51 | |
Sorry, you're right; I forgot importlib got it "right" while import got it "wrong". :) |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2020-01-23 17:51:31 | brett.cannon | set | messages: + msg360570 |
| 2020-01-22 23:53:41 | hroncok | set | messages: + msg360523 |
| 2020-01-22 18:36:49 | brett.cannon | set | messages: + msg360496 |
| 2020-01-21 21:17:23 | hroncok | set | messages: + msg360434 |
| 2020-01-21 21:12:28 | brett.cannon | set | messages: + msg360433 |
| 2020-01-21 12:01:13 | hroncok | set | nosy:
+ hroncok messages: + msg360394 |
| 2019-08-06 21:05:52 | brett.cannon | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2019-08-03 05:46:05 | miss-islington | set | nosy:
+ miss-islington messages: + msg348943 |
| 2019-07-29 18:27:33 | nanjekyejoannah | set | nosy:
- nanjekyejoannah |
| 2019-07-20 16:23:28 | rhettinger | set | nosy:
+ rhettinger messages: + msg348215 |
| 2019-07-20 15:36:00 | nsiregar | set | keywords:
+ patch stage: patch review pull_requests: + pull_request14657 |
| 2019-07-02 14:33:24 | nanjekyejoannah | set | nosy:
+ nanjekyejoannah |
| 2019-07-01 14:00:16 | ncoghlan | set | messages: + msg347021 |
| 2019-06-28 19:34:38 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka |
| 2019-06-28 19:28:02 | brett.cannon | set | messages: + msg346860 |
| 2019-06-28 19:26:57 | brett.cannon | set | messages: + msg346859 |
| 2019-06-28 19:26:11 | brett.cannon | set | messages: + msg346858 |
| 2019-06-28 19:22:13 | brett.cannon | create | |