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

[doc] First argument to raise can also be BaseException #90449

Closed
gtitze mannequin opened this issue Jan 7, 2022 · 16 comments
Closed

[doc] First argument to raise can also be BaseException #90449

gtitze mannequin opened this issue Jan 7, 2022 · 16 comments
Labels
3.9 3.10 3.11 docs Documentation in the Doc dir easy type-feature A feature request or enhancement

Comments

@gtitze
Copy link
Mannequin

gtitze mannequin commented Jan 7, 2022

BPO 46291
Nosy @aroberge, @vedgar, @sobolevn, @iritkatriel, @gtitze
PRs
  • gh-90449: Improve accuracy and readability of exceptions tutorial #31899
  • 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 = None
    closed_at = None
    created_at = <Date 2022-01-07.13:30:17.853>
    labels = ['easy', '3.9', '3.10', '3.11', 'type-feature', 'docs']
    title = '[doc] First argument to raise can also be BaseException'
    updated_at = <Date 2022-03-15.11:40:57.694>
    user = 'https://github.com/gtitze'

    bugs.python.org fields:

    activity = <Date 2022-03-15.11:40:57.694>
    actor = 'iritkatriel'
    assignee = 'docs@python'
    closed = False
    closed_date = None
    closer = None
    components = ['Documentation']
    creation = <Date 2022-01-07.13:30:17.853>
    creator = 'gtitze'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 46291
    keywords = ['patch', 'easy']
    message_count = 16.0
    messages = ['409961', '409988', '410001', '410005', '410008', '410013', '410027', '410028', '410035', '410036', '410037', '410057', '410060', '410064', '410123', '414773']
    nosy_count = 6.0
    nosy_names = ['aroberge', 'docs@python', 'veky', 'sobolevn', 'iritkatriel', 'gtitze']
    pr_nums = ['31899']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue46291'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    @gtitze
    Copy link
    Mannequin Author

    gtitze mannequin commented Jan 7, 2022

    The Python Tutorial describes the first argument to the raise statement as follows:

    """
    This must be either an exception instance or an exception class (a class that derives from Exception).
    """
    https://docs.python.org/3/tutorial/errors.html#raising-exceptions

    However, the Python Language Reference states, that exceptions should be a subclass or instance of BaseException.
    https://docs.python.org/3/reference/simple_stmts.html#grammar-token-python-grammar-raise_stmt

    I think it would be correct, to adapt the Tutorial to match the Reference.

    @gtitze gtitze mannequin assigned docspython Jan 7, 2022
    @gtitze gtitze mannequin added docs Documentation in the Doc dir 3.7 type-feature A feature request or enhancement 3.8 3.10 3.11 3.9 labels Jan 7, 2022
    @gtitze gtitze mannequin assigned docspython Jan 7, 2022
    @gtitze gtitze mannequin added docs Documentation in the Doc dir type-feature A feature request or enhancement labels Jan 7, 2022
    @sobolevn
    Copy link
    Member

    sobolevn commented Jan 7, 2022

    I think that this is intentional: tutorial gives some very basic ideas about what can be raised. Giving the whole context (you can raise two kinds of errors: Exception and BaseException, but they are different, should be used in different cases, and they are caught differently). Probably most users don't need to subclass BaseException at all.

    And if they need to: there are docs for that https://docs.python.org/3/library/exceptions.html#BaseException

    In my opinion - nothing should be changed there :)

    @iritkatriel
    Copy link
    Member

    iritkatriel commented Jan 7, 2022

    If you scroll up, you can see that BaseException was mentioned above:

    "All exceptions inherit from BaseException, and so it can be used to serve as a wildcard"

    so I don't think it will add confusion to fix this. And it will be more accurate.

    Gregor, would you like to submit a patch?

    @aroberge
    Copy link
    Mannequin

    aroberge mannequin commented Jan 7, 2022

    In https://docs.python.org/3/library/exceptions.html#Exception, it is written:

    "All built-in, non-system-exiting exceptions are derived from this class. All user-defined exceptions should also be derived from this class."

    Yes, technically, the root of all exceptions is BaseException. However, this seems to indicate that the advice given in the tutorial is correct.

    @iritkatriel
    Copy link
    Member

    iritkatriel commented Jan 7, 2022

    Andre, I don't follow. The OP is reporting an issue with a line about raising exceptions. Why is it correct to advise us to raise only Exception subclasses?

    @iritkatriel
    Copy link
    Member

    iritkatriel commented Jan 7, 2022

    There are other issues in this doc:

    1. The paragraph starting with "The except clause may specify a variable after the exception name.." appears after this face was used in an example. It should move up.

    2. In the User-defined Exceptions Section, there is a link to the Classes tutorial in the beginning "(See Classes for more about Python classes)", and another one at the end, "More information on classes it presented in chapter Classes". The second one can be removed.

    @aroberge
    Copy link
    Mannequin

    aroberge mannequin commented Jan 7, 2022

    Irit:

    Gregor indicates that, while the tutorial refers to "a class that derives from Exception", the Python Reference Language states exceptions should be subclasses of BaseException (which Exception *is*).

    You then invited Gregor to submit a patch to change the wording of the tutorial (implying that it would refer to a class that derives from BaseException).

    I pointed out that the advice given elsewhere is that user-defined exceptions should derive from Exception (thus, not from BaseException), so that nothing should be changed in the tutorial, and no change should be submitted.

    So, as I understand it: there is a strict hierarchy of exceptions classes, all inheriting from BaseException. However, user-defined exceptions should be derived from Exception (and not BaseException), as indicated in the tutorial and on the page describing what each exception indicates. If that is correct, nothing needs to be changed in the tutorial.

    @iritkatriel
    Copy link
    Member

    iritkatriel commented Jan 7, 2022

    Andre, are you saying that we should only RAISE Exception subclasses?

    @aroberge
    Copy link
    Mannequin

    aroberge mannequin commented Jan 7, 2022

    Irit:

    In all the books and tutorials I have seen, the advice is to try to catch specific exceptions whenever possible or, *at most*, to catch Exception (and not BaseException). This is to allow, for example, a program to be interrupted by a KeyboardInterrupt.

    As you know, the hierarchy is as follows:

    BaseException
    +-- SystemExit
    +-- KeyboardInterrupt
    +-- GeneratorExit
    +-- Exception
    +-- all others

    If specific action to do some cleanup before a SystemExit (usually the result of calling sys.exit()) or catching some KeyboardInterrupt (which is generally NOT done via a raise statement), then these specific exception should be caught.
    The documentation refers to GeneratorExit as not indicating an error needing to be caught by users.

    For this advice (catching Exception and not BaseException) to be correct, users should be advised (as they are in the tutorial) to raise exceptions derived from Exception (and not BaseException).

    @iritkatriel
    Copy link
    Member

    iritkatriel commented Jan 7, 2022

    For this advice (catching Exception and not BaseException) to be correct, users should be advised (as they are in the tutorial) to raise exceptions derived from Exception (and not BaseException).

    I disagree with that.

    @aroberge
    Copy link
    Mannequin

    aroberge mannequin commented Jan 7, 2022

    > For this advice (catching Exception and not BaseException) to be correct, users should be advised (as they are in the tutorial) to raise exceptions derived from Exception (and not BaseException).

    I disagree with that.

    You are a core developer and I just an end-user. I respect your expertise and will no longer comment.

    @gtitze
    Copy link
    Mannequin Author

    gtitze mannequin commented Jan 8, 2022

    Andre:

    You mention that user-defined exceptions should inherit from Exception. This is totally right and explicitly stated just a bit later in 8.6 on the same page of the tutorial. I think this perfectly covers this concern .

    However, the paragraph I refer to explains the raise statement and as stated in the reference, the raise statement must be followed by a class or instance derived from BaseException. Thus, I think it would just be accurate and people reading on don't stumble over this difference as I did.

    Regarding the mentioned wildcard: I think it wouldn't be a real wildcard anymore if it didn't catch ALL exceptions. Anyway the tutorial states that it needs to be used with extreme caution and the example re-raises the error.

    Irit:

    Yes I am happy provide a patch. I would also correct the other two issues you mentioned.

    @gtitze
    Copy link
    Mannequin Author

    gtitze mannequin commented Jan 8, 2022

    Irit:

    I would move the paragraph starting with "The except clause may specify a variable after the exception name ..." and the following example before the paragraph starting with "All exceptions inherit from BaseException, and so it can be used to serve as a wildcard ..."

    Or did you have another position in mind?

    @iritkatriel
    Copy link
    Member

    iritkatriel commented Jan 8, 2022

    Make a PR and we can then tweak it via code reviews, it will be easier.

    @vedgar
    Copy link
    Mannequin

    vedgar mannequin commented Jan 8, 2022

    Let me just say that I use raise SystemExit all the time. Beats from sys import exit, weird error messages about having to type parentheses, and surely beats "oh, am I on Windows so Ctrl+Z, or on Linux so Ctrl+D". :-]

    I also use raise KeyboardInterrupt sometimes in games, to test whether Ctrl+C handling works as expected at precise moments in the game (beats having to guess the millisecond in which to press Ctrl+C:).

    @iritkatriel
    Copy link
    Member

    iritkatriel commented Mar 8, 2022

    @gtitze - are you still planning to work on this?

    @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
    3.9 3.10 3.11 docs Documentation in the Doc dir easy type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants