Remove encouragement to author a base class for all Exception subclasses in a module

[d05aef1e-abc2-4a6e-b042-f2fd671dd9f2]

BPO	34538
Nosy	@malemburg, @gvanrossum, @freddrake, @brettcannon, @rhettinger, @methane, @nathanielmanistaatgoogle, @ammaraskar, @hugovk, @miss-islington
PRs	python/cpython#30361 python/cpython#30374 python/cpython#30375

^{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: ```python assignee = 'https://github.com/gvanrossum' closed_at = created_at = labels = ['easy', '3.9', '3.10', '3.11', 'type-feature', 'docs'] title = 'Remove encouragement to author a base class for all Exception subclasses in a module' updated_at = user = 'https://github.com/nathanielmanistaatgoogle' ``` bugs.python.org fields: ```python activity = actor = 'iritkatriel' assignee = 'gvanrossum' closed = True closed_date = closer = 'iritkatriel' components = ['Documentation'] creation = creator = 'Nathaniel Manista' dependencies = [] files = [] hgrepos = [] issue_num = 34538 keywords = ['patch', 'easy'] message_count = 18.0 messages = ['324286', '324302', '324312', '324317', '324344', '324388', '324390', '324392', '324405', '324408', '325069', '326771', '326772', '326804', '326805', '409629', '409636', '409637'] nosy_count = 11.0 nosy_names = ['lemburg', 'gvanrossum', 'fdrake', 'brett.cannon', 'rhettinger', 'methane', 'docs@python', 'Nathaniel Manista', 'ammar2', 'hugovk', 'miss-islington'] pr_nums = ['30361', '30374', '30375'] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue34538' versions = ['Python 3.9', 'Python 3.10', 'Python 3.11'] ```

[d05aef1e-abc2-4a6e-b042-f2fd671dd9f2]

https://docs.python.org/3.8/tutorial/errors.html (and all other versions of that page back at least as far as 2.7) currently contain the guidance "When creating a module that can raise several distinct errors, a common practice is to create a base class for exceptions defined by that module, and subclass that to create specific exception classes for different error conditions: \<code example>".

It may have seemed like a good idea at the time, but we now know from years of experience that this is an experiment that didn't pan out and we can consider this guidance a Now-Contraindicated Best Practice Of Yesteryear™.

Modules define subclasses of Exception for lots of reasons. Some of those subclasses have no relation to one another except that they are subclasses of Exception. Some of those subclasses define Exception instances that are never raised by code in the module, but that are expected to be raised by code passed to and called by the module.

Yes, there are times when a common base class may be appropriate - such as when an inheritance hierarchy and polymorphism that satisfy the Liskov Substitution Principle make sense for the Exception subclasses, and when the base class itself is used (such as when the base class is an item in the Raises: section of a function's doc string). But these cases aren't so common as to justify the advice being offered generally about all Exception subclass definitions.

Exception subclasses are just classes. Advising that authors may wish to define a common base class for all Exception subclasses in a module is like advising authors that they may wish to define a common base class for all object subclasses in a module - it's very, very, very occasionally a good idea in the particular circumstances of a particular module's implementation, but very generally not.

[methane]

I agree with you.

[rhettinger]

I think the advice should stay as-is. In my experience as a teacher, the possibility doesn't usually occur to people without it having been suggested. Also, it mirrors practices in the standard library (decimal.DecimalException and sqlite3.DatabaseError). For users, it provides a way to catch possible errors that are specific to a particular module.

[methane]

Also, it mirrors practices in the standard library (decimal.DecimalException and sqlite3.DatabaseError).

As Nathaniel said, "it's good idea in the particular circumstances of a particular module's implementation, but generally not." (I removed "very" because it is too noisy)

For example, socket.error was migrated into OSError. So not having base exception class for module is also "practices in the standard library".

For users, it provides a way to catch possible errors that are specific to a particular module.

For tutorial readers, caching errors specific to particular cause should be suggested, instead of particular module.

"How/When custom base exception class is useful" is very high level topic. It's too difficult for tutorial.

If tutorial reader start using base exception class without any real benefit, it will lead ugly code like this:

try: # some code here except ValueError as e: raise CustomValueError(e.args) except TypeError as e: raise CustomTypeError(e.args) except ...

[brettcannon]

I think the question is how often in real code to people want to catch all exceptions produced by a package or module but not any others. Perhaps it would be better to re-word the advice that when there are many related exceptions that it is suggested you have a base class for them?

[rhettinger]

See https://bugs.python.org/issue443559 and "git log -p 13af42822cd".

One other example from real code: requests.RequestException

[brettcannon]

I'm not questioning if people have ever created a base exception for an entire package, I'm just asking how often it's actually used when the base exception didn't make sense outside of the rule-of-thumb Nathaniel is pointing out?

For instance, it could makes sense in requests' case to have a base exception to help facilitate catching network-related errors but let through e.g. TypeError. But does that extend to most packages such that users regularly write an 'except' clause catching that package's common base exception type? That's my question and I personally don't have an answer beyond reflecting on this and not really remembering a case where I have (I _can_ remember following this pattern simply because it's habit at this point for some unknown reason :) .

[ammaraskar]

For some empirical data, I went through some popular packages that follow this pattern and searched for usage of their base exception classes:

Requests: https://github.com/search?q=except+requests.RequestException&type=Code

PyYaml: https://github.com/search?q=except+yaml.YAMLError&type=Code

Jinja2: https://github.com/search?q=%22except+jinja2.TemplateError%22&type=Code

https://github.com/search?q=%22except+jinja2.exceptions.TemplateError%22&type=Code

https://github.com/search?q=%22except+TemplateError%22&type=Code

[methane]

I didn't claim this pattern is not used anymore. My point is "should we suggest this pattern for tutorial readers?"

• Is this pattern recommended blindly? -- I think no.
• How / when this pattern is recommended? -- When there is use case people want to catch the base exception.
• Should it be in tutorial? -- I doubt it. This topic requires some experience to understand.

[methane]

https://github.com/search?q=%22except+TemplateError%22&type=Code

For example, I found flask_mako's TemplateException in this search result.

Strictly speaking, this is not base exception class. It is wraps exception during template rendering. But "why this class is useful?" is very similar to base exception class.

It provides better traceback for mako template. They use this class for "particular reason", not because it's generally recommended practice.

And this "particular reason" shouldn't be in Python tutorial, clearly.

---

If we really need exception class hierarchy in tutorial, I think OSError is the best example. When opening a file, `except OSError:` is much better than `except (PermissionError, FileNotFound, ...)`. It's the most clear example when common base class for some distinct exceptions is useful.

[d05aef1e-abc2-4a6e-b042-f2fd671dd9f2]

I’d like to try to steer this conversation back toward what I think is the actionable question: “does the exemplification of this practice in the Errors and Exceptions portion of The Python Tutorial bring about a net benefit or a net cost to its intended audience?”.

What is the benefit? It is that when an author happens to be authoring a module *all user-defined Exception subclasses of which satisfy some more-specific-than-Exception type* the author is led to define an intermediate class between Exception and the directly-used user-defined Exception subclasses capturing that type in a usable code element.

What readers of the tutorial enjoy this benefit? Pretty much only those authors (who happen to be authoring a module *all user-defined Exception subclasses of which satisfy some more-specific-than-Exception type) who are still learning about classes and inheritance. That’s a doubly slim population, isn’t it? Maybe also those who kind of know, but aren’t so sure and could use some reinforcement from seeing in the tutorial something that they independently did on their own in their own code. I wouldn’t think that authors who already know with confidence and from experience about classes and inheritance would benefit from the example in the tutorial, so “In my experience as a teacher, the possibility doesn't usually occur to people without it having been suggested” comes as a surprise to me. But… okay, them too - but again, only when they happen to be authoring a module *all user-defined Exception subclasses of which satisfy some more-specific-than-Exception type.

What is the cost? It is that when an author happens to be authoring a module that does *not* have the property that all user-defined Exception subclasses satisfy some more-specific-than-Exception type, the common intermediate class is just bloat. It’s noise disrupting the signal. It undermines the API design advice “when in doubt, leave it out”. It unnecessarily widens the module’s API. It undermines the API design advice “classes are the most complex kind of code element in common use so they have the highest value proposition to satisfy to justify their existence”. It violates the Liskov Substitution Principle. Maybe most importantly, it obscures other relationships among the user-defined Exception subclasses in the module such as a superclass shared by some-but-not-all of the module’s user-defined Exception subclasses, if such other relationships exist.

What readers of the tutorial pay this cost? Those who are still learning the language. And those who are following pattern and convention - note that the tutorial contains only one example of user-defined Exception subclasses, and… an unfortunate fact of life of tutorials is that readers are invited to generalize from single examples. And, as I think we’ve seen in this conversation, those who just picked up the practice at one point and have kept it going.

The Exception subclass hierarchy of sqlite3 that was mentioned earlier in this conversation demonstrates exactly this bloat and misapplication - there’s Warning, which is a direct subclass of Exception, and there’s Error, which is also a direct subclass of Exception and has the erroneous specification “The base class of the other exceptions in this module”, and there’s DatabaseError, which is a subclass of Error, and then there are IntegrityError, ProgrammingError, OperationalError, and NotSupportedError, which inherit from DatabaseError. What’s the use of Error? There are no code elements in the module specified as raising Error. There’s example code in the module showing “except sqlite3.Error”, but why shouldn’t that be “except sqlite3.DatabaseError”?

It’s a red herring is that the practice appears widely applied in existing Python code - well of course; it’s been exemplified in the tutorial for seventeen years! :-P

One last thing to consider: look at the example specifically - InputError and TransitionError. There’s no elsewhere-in-the-module example code showing a function that has “Error” in its “Raises:” specification and could raise either an InputError or a TransitionError, and there’s no outside-of-the-module example code showing a user of the module calling a module function and saving duplicated lines of code because they want to respond to InputErrors and TransitionErrors in exactly the same way.

We should remove the “Base class for exceptions in this module” Error class from the tutorial’s example because it just isn’t beneficial enough, in enough applicable modules, to enough authors, and it’s more than costly enough, in enough modules to which it doesn’t apply, and to enough authors, even just as noise and API bloat. I don’t know that this could have been calculated from first principles seventeen years ago; I think perhaps it took the experience of having the guidance out there, so rarely merited and so widely implemented, to see it being a net drag.

[methane]

Thanks, Nathaniel. I totally concur with you.

This is tutorial section. We should focus on readers of the tutorial.

[rhettinger]

Guido, the check-in message for this section indicates that Fred Drake added this wording at your behest. Do you still agree with the guidance and examples or would you like to have it removed from all active versions of the documentation as proposed?

https://docs.python.org/3.8/tutorial/errors.html#user-defined-exceptions

[gvanrossum]

I think as a general recommendation it is not such a good idea that we should specifically mention it. (Is it in PEP-8 too? If so it should be removed there too.)

It's a pattern that is sometimes helpful, sometimes not. I don't think that people need to hear about it from the official docs about exceptions. People can learn from the standard exception hierarchy that sometimes it's useful to have a common base class *for exceptions that are related in some way*, in particular if there would be a reason to catch all of them with the same handler.

So I'm in agreement with Nathaniel M here.

[malemburg]

Just as extra data point:

It is fairly common to have a common exception class which is then used a mixin class together with the standard exception classes, so that you can indeed identify the source of an exception and catch errors based on the source (e.g. say you want to catch database errors coming from MySQL specifically).

The Python DB-API also requires to create a separate hierarchy for this purpose.

Overall, I wouldn't call this a non-best practice. It depends on the use case, whether it's useful or not.

[miss-islington]

New changeset 2db56130631255ca2eb504519430fb2f1fe789e9 by Hugo van Kemenade in branch 'main': bpo-34538: Remove Exception subclassing from tutorial (GH-30361) https://github.com/python/cpython/commit/2db56130631255ca2eb504519430fb2f1fe789e9

[miss-islington]

New changeset 0b3c3cbbaf2967cc17531d65ece0969b0d2a2079 by Miss Islington (bot) in branch '3.10': bpo-34538: Remove Exception subclassing from tutorial (GH-30361) https://github.com/python/cpython/commit/0b3c3cbbaf2967cc17531d65ece0969b0d2a2079

[miss-islington]

New changeset 4affb996ce6353dd029ece0c7d36f7c7c0af2de3 by Miss Islington (bot) in branch '3.9': bpo-34538: Remove Exception subclassing from tutorial (GH-30361) https://github.com/python/cpython/commit/4affb996ce6353dd029ece0c7d36f7c7c0af2de3