This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Remove encouragement to author a base class for all Exception subclasses in a module
Type: enhancement Stage: resolved
Components: Documentation Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gvanrossum Nosy List: Nathaniel Manista, ammar2, brett.cannon, docs@python, fdrake, gvanrossum, hugovk, lemburg, methane, miss-islington, rhettinger
Priority: normal Keywords: easy, patch

Created on 2018-08-28 20:40 by Nathaniel Manista, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 30361 merged hugovk, 2022-01-03 17:27
PR 30374 merged miss-islington, 2022-01-03 22:52
PR 30375 merged miss-islington, 2022-01-03 22:52
Messages (18)
msg324286 - (view) Author: Nathaniel Manista (Nathaniel Manista) Date: 2018-08-28 20:40
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.
msg324302 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-08-29 07:09
I agree with you.
msg324312 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-08-29 09:38
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.
msg324317 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-08-29 10:20
> 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 ...
msg324344 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2018-08-29 19:17
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?
msg324388 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-08-30 15:54
See https://bugs.python.org/issue443559 and "git log -p 13af42822cd".

One other example from real code:  requests.RequestException
msg324390 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2018-08-30 16:10
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 :) .
msg324392 - (view) Author: Ammar Askar (ammar2) * (Python committer) Date: 2018-08-30 16:34
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
msg324405 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-08-31 07:19
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.
msg324408 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-08-31 08:48
> 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.
msg325069 - (view) Author: Nathaniel Manista (Nathaniel Manista) Date: 2018-09-11 21:36
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.
msg326771 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-10-01 08:53
Thanks, Nathaniel.  I totally concur with you.

This is tutorial section.  We should focus on readers of the tutorial.
msg326772 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-10-01 09:10
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
msg326804 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-10-01 15:14
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.
msg326805 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2018-10-01 15:22
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.
msg409629 - (view) Author: miss-islington (miss-islington) Date: 2022-01-03 22:52
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
msg409636 - (view) Author: miss-islington (miss-islington) Date: 2022-01-03 23:10
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
msg409637 - (view) Author: miss-islington (miss-islington) Date: 2022-01-03 23:19
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
History
Date User Action Args
2022-04-11 14:59:05adminsetgithub: 78719
2022-01-03 23:21:11iritkatrielsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2022-01-03 23:19:33miss-islingtonsetmessages: + msg409637
2022-01-03 23:10:23miss-islingtonsetmessages: + msg409636
2022-01-03 22:52:22miss-islingtonsetpull_requests: + pull_request28586
2022-01-03 22:52:18miss-islingtonsetpull_requests: + pull_request28585
2022-01-03 22:52:17miss-islingtonsetnosy: + miss-islington
messages: + msg409629
2022-01-03 22:49:34iritkatrielsetversions: + Python 3.11, - Python 3.8
2022-01-03 17:27:31hugovksetkeywords: + patch
nosy: + hugovk

pull_requests: + pull_request28575
stage: patch review
2021-03-14 17:34:29iritkatrielsetkeywords: + easy
versions: + Python 3.9, Python 3.10, - Python 2.7, Python 3.4, Python 3.5, Python 3.6, Python 3.7
2018-10-01 15:22:07lemburgsetnosy: + lemburg
messages: + msg326805
2018-10-01 15:14:07gvanrossumsetmessages: + msg326804
2018-10-01 09:10:12rhettingersetassignee: docs@python -> gvanrossum

messages: + msg326772
nosy: + gvanrossum
2018-10-01 08:53:42methanesetmessages: + msg326771
2018-09-11 21:36:26Nathaniel Manistasetmessages: + msg325069
2018-08-31 08:48:49methanesetmessages: + msg324408
2018-08-31 07:19:44methanesetmessages: + msg324405
2018-08-30 16:34:36ammar2setnosy: + ammar2
messages: + msg324392
2018-08-30 16:10:36brett.cannonsetmessages: + msg324390
2018-08-30 15:54:19rhettingersetnosy: + fdrake
messages: + msg324388
2018-08-29 19:17:05brett.cannonsetnosy: + brett.cannon
messages: + msg324344
2018-08-29 10:20:13methanesetmessages: + msg324317
2018-08-29 09:38:44rhettingersetnosy: + rhettinger
messages: + msg324312
2018-08-29 07:09:23methanesetnosy: + methane
messages: + msg324302
2018-08-28 20:40:51Nathaniel Manistacreate