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

add ModuleNotFoundError #59971

Closed
ericsnowcurrently opened this issue Aug 22, 2012 · 64 comments
Closed

add ModuleNotFoundError #59971

ericsnowcurrently opened this issue Aug 22, 2012 · 64 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@ericsnowcurrently
Copy link
Member

BPO 15767
Nosy @gvanrossum, @warsaw, @brettcannon, @theller, @jcea, @pitrou, @ned-deily, @ezio-melotti, @asvetlov, @cjerdonek, @ericsnowcurrently, @serhiy-storchaka, @gvanrossum
Dependencies
  • bpo-18200: Update stdlib to use ModuleNotFoundError
  • 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/ericsnowcurrently'
    closed_at = <Date 2016-09-07.23:58:40.759>
    created_at = <Date 2012-08-22.19:45:17.750>
    labels = ['interpreter-core', 'type-feature']
    title = 'add ModuleNotFoundError'
    updated_at = <Date 2018-02-27.13:48:13.030>
    user = 'https://github.com/ericsnowcurrently'

    bugs.python.org fields:

    activity = <Date 2018-02-27.13:48:13.030>
    actor = 'cwg'
    assignee = 'eric.snow'
    closed = True
    closed_date = <Date 2016-09-07.23:58:40.759>
    closer = 'eric.snow'
    components = ['Interpreter Core']
    creation = <Date 2012-08-22.19:45:17.750>
    creator = 'eric.snow'
    dependencies = ['18200']
    files = []
    hgrepos = []
    issue_num = 15767
    keywords = []
    message_count = 64.0
    messages = ['168906', '175638', '175650', '175651', '175659', '175660', '175674', '175699', '175703', '181901', '181914', '181919', '181926', '182260', '182263', '182271', '182322', '182332', '182339', '182385', '182396', '182401', '182404', '182409', '182411', '182414', '182415', '182437', '182455', '182459', '182466', '191046', '191054', '191055', '192095', '192139', '192142', '192143', '192161', '192172', '192176', '192189', '192199', '192200', '192201', '192202', '192207', '192209', '192261', '192263', '192318', '219127', '274866', '274921', '312930', '312940', '312949', '312965', '312969', '312971', '312996', '313004', '313006', '313008']
    nosy_count = 17.0
    nosy_names = ['gvanrossum', 'barry', 'brett.cannon', 'theller', 'jcea', 'pitrou', 'ned.deily', 'ezio.melotti', 'Arfrever', 'cvrebert', 'asvetlov', 'chris.jerdonek', 'python-dev', 'eric.snow', 'serhiy.storchaka', 'Guido.van.Rossum', 'cwg']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue15767'
    versions = ['Python 3.6']

    @ericsnowcurrently
    Copy link
    Member Author

    In bpo-15316 (msg168896, Brett Cannon):

    Create a ModuleNotFoundError exception that subclasses ImportError
    and catch that (breaks doctests and introduces a new exception that
    people will need to be aware of, plus the question of whether it
    should just exist in importlib or be a builtin)

    While it's too late to go into 3.3, this is a reasonable addition for 3.4. Perhaps other ImportError subclasses are warranted, but they can be addressed separately.

    @ericsnowcurrently ericsnowcurrently added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Aug 22, 2012
    @brettcannon
    Copy link
    Member

    Should it be ModuleNotFoundError or ModuleNotFound? It's not necessarily an error if a module doesn't exist, so failing to find it isn't quite that negative. It also acts somewhat like StopIteration/GeneratorExit which also don't have the common "Error" suffix of exception names.

    @cjerdonek
    Copy link
    Member

    Since it subclasses ImportError, it seems like we're already considering it to be an error? StopIteration and GeneratorExit don't inherit from an "Error" exception type unlike, say, the OSError exception types.

    @cjerdonek
    Copy link
    Member

    I meant to include this link for convenience:

    http://docs.python.org/dev/library/exceptions.html#exception-hierarchy

    @ericsnowcurrently
    Copy link
    Member Author

    I'd say ModuleNotFoundError. You could argue that other exception types aren't really errors in certain circumstances. I'd think that generally this would be an error.

    @ericsnowcurrently
    Copy link
    Member Author

    Effectively the exception indicates that the import system had an error.

    @ezio-melotti
    Copy link
    Member

    I prefer ModuleNotFound. Its meaning is already clear enough, even without the Error suffix.
    OTOH we now have FileNotFoundError, but all the other OSError subclasses have that suffix.
    In general I think the suffix is necessary when it's not already clear from the name that we are dealing with an exception, otherwise it can be dropped.

    @cjerdonek
    Copy link
    Member

    To state more explicitly the observation I alluded to in my comment above, we currently follow (without exception -- pun intended :) ) the convention that subclasses of exceptions that end in "Error" also end in "Error". We also do the same with the suffix "Warning".

    The latter is another reason to include the suffix "Error" -- to eliminate ambiguity as to whether the exception type inherits from a Warning class (e.g. from ImportWarning).

    @ezio-melotti
    Copy link
    Member

    That seems indeed to be the case with built-in exceptions. I'm not sure if it's intentional or just a coincidence. I agree that warnings should always have a "Warning" suffix to distinguish them from exceptions, but in the stdlib the "Error" suffix is not used consistently. There are exceptions like: FloatOperation, DivisionByZero, InvalidOperation, TimeoutExpired, BrokenProcessPool, BufferTooShort, ImproperConnectionState, UnknownProtocol, InvalidURL, etc..
    Anyway I don't have a strong opinion about this, so if you think the name should be ModuleNotFoundError it's OK with me (i.e. I'm -0).

    @warsaw
    Copy link
    Member

    warsaw commented Feb 11, 2013

    For me, it mostly comes down to whether end-users are expected to see such errors generally or not. We see ImportErrors all the time, and they are clearly errors. If we're expected to see and deal with MNF, and if in such cases it's generally considered an error, then MNFError is better. If it's mostly an internal/hidden thing, then MNF doesn't bother me.

    @brettcannon
    Copy link
    Member

    Right, so what's typical? =) I mean do most people see ImportError for optional modules (e.g. not on support platforms), or do most people see ImportError because they messed up and tried to import something that they expected but actually isn't there for some reason.

    @warsaw
    Copy link
    Member

    warsaw commented Feb 11, 2013

    On Feb 11, 2013, at 05:31 PM, Brett Cannon wrote:

    Right, so what's typical? =) I mean do most people see ImportError for
    optional modules (e.g. not on support platforms), or do most people see
    ImportError because they messed up and tried to import something that they
    expected but actually isn't there for some reason.

    There are a few common use cases (or perhaps anti-use cases) where you see
    ImportErrors. I might be missing some, but I'd put these in roughly
    descending order of commonness.

    • Trying alternative imports for compatibility reasons. You always expect
      ImportErrors in these cases, and you'll always catch them in try/excepts.

    • Missing modules, submodules, or attributes in from-imports. These can be
      unexpected if you think you've got the right version of a package, or
      expected for compatibility reasons.

    • Trying to conditionally import optional modules. Again, expected, and
      they'll be wrapped in try/except.

    I guess the case you're trying to differentiate with MNF is, the from-import
    case, i.e. did the error occur because the module was missing or because the
    attribute was missing?

    It's hard to say which is more likely, which I guess is why you're having a
    hard time deciding. :) If I had to vote, I'd go with MNFError 1) because it's
    a subclass of ImportError; 2) it'll be more informative in the case where it
    really *is* an error; 3) isn't that big of a deal in cases where it's
    expected; 4) we're used to seeing ImportError anyway, and probably most code
    won't care and will just use ImportError.

    @brettcannon
    Copy link
    Member

    Screw it, I'll go with ModuleNotFoundError since it is a subclass of ImportError and so it might come off as weird as saying the superclass is an "Error" but the subclass is not.

    @ericsnowcurrently
    Copy link
    Member Author

    +1 on just getting it done with ModuleNotFoundError. FWIW, I'd be glad to do it. I'm taking a self-imposed break from the nearly finished C-OrderedDict!

    @serhiy-storchaka
    Copy link
    Member

    from foo import bar

    Here bar can be not module, but an attribute of foo (for example, os.path). What error will be raised in this case? Module or attribute - this is an implementation detail; why do we distinguish between these cases?

    @brettcannon
    Copy link
    Member

    Eric: knock yourself out. =)

    Serhiy: What exception is raised in that situation is controlled by the eval loop, not importlib so that would be a separate change. But regardless, there is no way to infer whether you expected an attribute or module to be there, just that you were after something that didn't exist. But I would argue most people import at the module level and not the attribute level, and so raising an ModuleNotFoundError would be acceptable.

    @pitrou
    Copy link
    Member

    pitrou commented Feb 18, 2013

    I've lost track of why this new exception was needed. Could someone provide a summary? Thanks :-)

    @brettcannon
    Copy link
    Member

    TL;DR for Antoine: when using a fromlist, import failures from that list are silently ignored. Because ImportError is overloaded in terms of what it means (e.g. bag magic number, module not found) there needs to be a clean way to tell the import failed because the module wasn't found so other ImportError reasons can properly propagate.

    @cjerdonek
    Copy link
    Member

    > from foo import bar
    > Here bar can be not module, but an attribute of foo (for example, os.path).
    Serhiy: What exception is raised in that situation is controlled by the eval loop, not importlib so that would be a separate change.

    Just to clarify from this exchange, is there a chance we will use this same exception type (perhaps in a later change) in cases where bar is not found? If so, I think it's worth considering something like "NotFoundImportError" or "ImportNotFoundError" that doesn't single out module. Importing classes, etc. is quite a common pattern (e.g. examples appear in PEP-8).

    @brettcannon
    Copy link
    Member

    The original need was for internal importlib usage, but upon reflection it could also be used by the eval loop for that (http://hg.python.org/cpython/file/83d70dd58fef/Python/ceval.c#l4560), so I'm fine with changing the name to ImportNotFoundError.

    @brettcannon brettcannon changed the title add ModuleNotFoundError add ImportNotFoundError Feb 19, 2013
    @pitrou
    Copy link
    Member

    pitrou commented Feb 19, 2013

    The original need was for internal importlib usage, but upon
    reflection it could also be used by the eval loop for that
    (http://hg.python.org/cpython/file/83d70dd58fef/Python/ceval.c#l4560),
    so I'm fine with changing the name to ImportNotFoundError.

    I don't understand what ImportNotFoundError means: an import was
    not found? ModuleNotFoundError was obvious.

    @brettcannon
    Copy link
    Member

    Technically it should be ModuleOrSomeObjectNotFoundBecauseFromListIsTheBaneOfMyExistenceError, but we might be starting to mix paints for paints a shed shortly.

    Fine, that's 1 to 1 for ModuleNotFoundError vs. ImportNotFoundError.

    @cjerdonek
    Copy link
    Member

    If we can promise not to use it in the from-import case :) I'm okay with the more specific name (in fact it is preferable). From Brett's response, it sounds like we have flexibility there and don't need it to be the same? For from-import I would prefer the generic ImportError or adding a new type between ImportError and ModuleNotFoundError (like ImportNotFoundError) over using a name that is not entirely correct.

    @serhiy-storchaka
    Copy link
    Member

    Can this be the same ImportError but with special flag?

    @warsaw
    Copy link
    Member

    warsaw commented Feb 19, 2013

    On Feb 19, 2013, at 07:24 PM, Serhiy Storchaka wrote:

    Can this be the same ImportError but with special flag?

    Like an attribute on the exception? +1

    @pitrou
    Copy link
    Member

    pitrou commented Feb 19, 2013

    >Can this be the same ImportError but with special flag?

    Like an attribute on the exception? +1

    ImportError.has_different_meaning_but_too_lazy_to_create_a_distinct_exception_class_for_it ?

    (or perhaps you would prefer the camelCase version :-))

    @brettcannon
    Copy link
    Member

    Chris: Having a more generic name for import-from at the eval loop level on top of NoModuleFoundError is breaking the "practicality over purity" rule. ImportSearchFailed might be the closest we can come to a generic name for what occurred.

    Serihy & Barry: no. We do that now and it's already a nasty little hack. It would be better to let people catch an exception signaling that an import didn't happen because some module is missing than require introspection on a caught ImportError to tell what is going on (there's a reason why Antoine went to all of that trouble to add new exceptions so we don't have to look at the errno attribute on OSError). Exceptions are structured to work off of inheritance hierarchies (says the man who co-wrote the PEP to make all PEPs inherit from BaseException).

    @serhiy-storchaka
    Copy link
    Member

    If most user code should catch a ModuleNotFoundError exception, perhaps it will be better rename old ImportError to ImportlibError (or ImportingError, or ImportMachineryError, or BaseImportError) and new ModuleNotFoundError to ImportError. This will left most existing user code untouched.

    @warsaw
    Copy link
    Member

    warsaw commented Jul 2, 2013

    On Jul 02, 2013, at 07:16 AM, Serhiy Storchaka wrote:

    If most user code should catch a ModuleNotFoundError exception, perhaps it
    will be better rename old ImportError to ImportlibError (or ImportingError,
    or ImportMachineryError, or BaseImportError) and new ModuleNotFoundError to
    ImportError. This will left most existing user code untouched.

    I urge some caution here. I don't know that the above will cause problems,
    but I do think you're walking into PEP territory. I would feel much more
    comfortable with an analysis based on testing existing third party code.

    @brettcannon
    Copy link
    Member

    OK, I'll revert the changes related to ModuleNotFoundError.

    As for adding a 'reason' attribute, I see two sticking points. One is how to set the enum value. There is both the C code issue (specifically so ceval.c and import.c can use the values) as well as importlib's bootstrapping restrictions. I'll have to think about whether there is any reasonable way to make it work.

    Second, as you hinted at Guido, is exactly what the acceptable cases may be. I would probably start with any ImportError raised directly by import itself and nothing more. Other things from loaders (e.g. bad magic number, stale bytecode, etc.) could be initially left out. That would mean the following possible values:

    • module is not a package
    • module not found
    • None in sys.modules

    But the bootstrapping issues for the enum module is probably going to be the showstopper for this. That suggests either adding not_found or figuring out some way to prevent _not_found from leaking outside of importlib (which I now think I can do somewhat reasonably).

    @pitrou
    Copy link
    Member

    pitrou commented Jul 2, 2013

    But the bootstrapping issues for the enum module is probably going
    to be the showstopper for this.

    Have we succumbed to the enum religion already? Just make it a plain string.

    @brettcannon
    Copy link
    Member

    In this instance where there are only a set number of options are expected to be officially valid, yes I think enums are a good fit.

    As for strings, the only way I would be okay with that is defining the strings either as attributes on ImportError itself or off of importlib to make it easy to do a comparison. But in that case I might as well just drop _not_found and use str(exc).startswith('No module named ') to detect what I need and be done with it.

    @pitrou
    Copy link
    Member

    pitrou commented Jul 2, 2013

    In this instance where there are only a set number of options are
    expected to be officially valid, yes I think enums are a good fit.

    They are a good fit, that doesn't mean they're the only one.

    As for strings, the only way I would be okay with that is defining
    the strings either as attributes on ImportError itself or off of
    importlib to make it easy to do a comparison.

    What does that mean?
    I don't understand how exc.reason == 'module_not_found' is
    harder than exc.reason == ImportReason.MODULE_NOT_FOUND.

    @brettcannon
    Copy link
    Member

    It's not a question of harder but more error-prone since a typo in the string won't be directly noticed while mistyping an attribute name will be.

    @pitrou
    Copy link
    Member

    pitrou commented Jul 2, 2013

    It's not a question of harder but more error-prone since a typo in
    the string won't be directly noticed while mistyping an attribute
    name will be.

    Ok, agreed. I guess it's ok, if it only adds one or two attributes.

    @ericsnowcurrently
    Copy link
    Member Author

    I'm sorry, but this seems like it should be an importlib internal
    affair. The new exception is too much in everyone's face, because
    the exception name gets printed on every traceback.

    That's the crux of the issue. If there isn't much utility outside importlib to distinguishing between module-not-found and other causes of ImportError, then there isn't much point to a new exception. It just boils down to what the other potential causes of ImportError are and how much people care about them.

    I keep thinking about PEP-3151 (IOError/OSError hierarchy rework) and the lessons we've learned about exception attributes vs. subclasses. For readability and write-ability, I'd rather write this:

    try:
    from _collections import OrderedDict
    except ModuleNotFoundError:
    pass

    than this:

    try:
    from _collections import OrderedDict
    except ImportError as e:
    if e.reason is not importlib.machinery.ImportReason.MODULE_NOT_FOUND:
    raise

    But the relevant question is, what is the benefit (outside importlib) of either over this:

    try:
    from _collections import OrderedDict
    except ImportError:
    pass

    @gvanrossum
    Copy link
    Member

    Right. Outside importlib there shouldn't be a need to distinguish between the cases (especially given that the exception works differently than its name suggests).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 4, 2013

    New changeset 0e4e062751fa by Brett Cannon in branch 'default':
    Issue bpo-15767: Back out 8d28d44f3a9a related to ModuleNotFoundError.
    http://hg.python.org/cpython/rev/0e4e062751fa

    New changeset e3ec8b176a80 by Brett Cannon in branch 'default':
    Issue bpo-15767: Revert 3a50025f1900 for ModuleNotFoundError
    http://hg.python.org/cpython/rev/e3ec8b176a80

    New changeset ee9662d77ebb by Brett Cannon in branch 'default':
    Issue bpo-15767: back out 8a0ed9f63c6e, finishing the removal of
    http://hg.python.org/cpython/rev/ee9662d77ebb

    New changeset de947db308ba by Brett Cannon in branch 'default':
    Issue bpo-15767: Excise the remaining instances of ModuleNotFoundError
    http://hg.python.org/cpython/rev/de947db308ba

    @ericsnowcurrently
    Copy link
    Member Author

    Any chance we could revive ModuleNotFoundError? It's nice to be able to distinguish between the failure to *find* the module during import from other uses of ImportError. I'd definitely expect it to work the way Guido explained. Basically only importlib._bootstrap._find_and_load_unlocked() would raise ModuleNotFoundError (when _find_spec() returns None).

    I've found the exception to be very useful while working on the importlib backport (https://bitbucket.org/ericsnowcurrently/importlib2). My desire for adding ModuleNotFoundError is unrelated to its internal use in importlib that motivated the original request (see msg182332).

    Here's the signature:

    ModuleNotFoundError(*args, name=None), inherits from ImportError

    For reference, here's ImportError:

      ImportError(*args, name=None, path=None)

    ModuleNotFoundError would need to be exposed somewhere sensible since once people see in tracebacks they'll want to catch it. :) I'd expect that to be either in builtins or as importlib.ModuleNotFoundError. We may be able to get away with not adding it to builtins, but maybe it would still make sense.

    @gvanrossum
    Copy link
    Member

    +1 to add this to 3.6b1.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 7, 2016

    New changeset 90549c3c609c by Eric Snow in branch 'default':
    Issue bpo-15767: Add ModuleNotFoundError.
    https://hg.python.org/cpython/rev/90549c3c609c

    New changeset 5fdb8c897023 by Eric Snow in branch 'default':
    Issue bpo-15767: Use ModuleNotFoundError.
    https://hg.python.org/cpython/rev/5fdb8c897023

    @cwg
    Copy link
    Mannequin

    cwg mannequin commented Feb 26, 2018

    I'm trying to understand why ModuleNotFoundError was added to 3.6. The "what's new" entry links to this page.

    Looking at the discussion, Guido said in 2013: "Right. Outside importlib there shouldn't be a need to distinguish between the cases (especially given that the exception works differently than its name suggests).". Then, three years later, he supports the inclusion of the patch, without that any new arguments had been given.

    Could someone explain (or link to) what happened inbetween?

    @gvanrossum
    Copy link
    Member

    Read Eric's message before mine.

    @cwg
    Copy link
    Mannequin

    cwg mannequin commented Feb 26, 2018

    Read Eric's message before mine.

    Of course I read it, I wouldn't have asked otherwise. Eric mentions an older message ("see msg182332") that *predates* your judgment that "outside importlib there shouldn't be a need to distinguish between the cases". Otherwise Eric says that he finds "the exception to be very useful", but frankly, I don't see why (outside of importlib or backports of it). What changed since msg192263?

    @gvanrossum
    Copy link
    Member

    I don't like the way you're asking questions here. If you're interested
    just as a historian of the language, it will have to wait. If you're
    questioning the decision, please come out and say so.

    @cwg
    Copy link
    Mannequin

    cwg mannequin commented Feb 26, 2018

    My curiosity was piqued when I saw ModuleNotFoundError, so I decided to look it up. This led me to this page and I read the complete discussion. I still did not understand the decision, so I allowed myself to ask, also because I believe that when new features are introduced it should be clear what they are good for. That's all.

    @cwg
    Copy link
    Mannequin

    cwg mannequin commented Feb 26, 2018

    In the above, please replace "understand the decision" by "understand the usefulness of it".

    In the above discussion, as an alternative to a new exception, it was proposed to add an attribute to ImportError ('reason'), but then people seemed to agree that this is quite useless outside of importlib (msg192261). But then I don't understand why the original idea of the exception was revived.

    @cjerdonek
    Copy link
    Member

    Eric touched on the use when he said the following above:

    It's nice to be able to distinguish between the failure to *find* the module during import from other uses of ImportError.

    To make up one example, you might want to use a fallback module if a package isn't installed:

    try:
        from fancy_parser import NewParser as HTMLParser
    except ModuleNotFoundError:
        from html.parser import HTMLParser
    

    But you might still want an error if the package is installed, though incorrectly (e.g. fancy_parser is installed, but an old version that doesn't have NewParser). Catching ImportError would swallow this error, whereas ModuleNotFoundError would let it bubble up.

    @cwg
    Copy link
    Mannequin

    cwg mannequin commented Feb 27, 2018

    Thank you, Chris, for your reply. If this is indeed the main use case of ModuleNotFoundError, I respectfully suggest to document it better. The way things are now, Python users who switch to 3.6 encounter this new exception during their work with the interpreter and invariably wonder "Should I change anything in my code because of this? If not, why was it introduced?". In my opinion the current documentation does not answer these questions well. Note that this is not about some deeply buried detail. Every Python programmer is bound to encounter this.

    That said, I cannot imagine many cases where user code would like to fall back to html.parser only if fancy_parser is not installed but not if an older version of fancy_parser is installed (or maybe it's an entirely *different* fancy_parser?). And in the rare cases where that is desired, it was already perfectly idiomatic to do so:

    try:
        import fancy_parser
    except ImportError:
        from html.parser import HTMLParser
    else:
        from fancy_parser import NewParser as HTMLParser
    

    @ned-deily
    Copy link
    Member

    Christoph, thanks for your suggestion. If you think the documentation needs improving, please open a new issue with any suggested wording (or, even better, a doc PR). This issue (bpo-15767) has long been closed and any discussion here is likely to not be acted on.

    @cwg
    Copy link
    Mannequin

    cwg mannequin commented Feb 27, 2018

    Unfortunately I do not feel competent enough to submit a documentation patch because I still do not understand why ModuleNotFoundError was added.

    I don't want to bother you further with this. Thank you all for your prompt replies. If you agree with me that there is indeed an issue, please open it yourself.

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    9 participants