Skip to content

DOC: add section about how to write good error messages #25292

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.

Sign up for GitHub

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

Open
tacaswell wants to merge 1 commit into
base: main
Choose a base branch
from
Open

DOC: add section about how to write good error messages #25292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ galleries/tutorials/intermediate/CL02.png
# sphinx build directory
doc/_build
doc/api/_as_gen
doc/devel/_as_gen
# autogenerated by sphinx-gallery
doc/examples
doc/gallery
Expand Down
55 changes: 55 additions & 0 deletions doc/devel/coding_guide.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,58 @@ and running the same script will display
:hidden:

license.rst


.. _verbose-error-messages:

Error Messages
--------------


Error messages should err on the side of being verbose, descriptive, and
specific about what went wrong. They should be informative enough so that a
typical user (not an expert of our API) can understand, debug, and fix their
problem based solely on the message, without the need to consult the online
documentation.

For example, input validation errors should include, where possible,
information about which input is invalid, what value was passed in, and why the
value is invalid, e.g. ::

raise ValueError(f"{value=!r} is invalid, value must be a positive integer")
Copy link
Member

@timhoffm timhoffm Oct 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to include the parameter name. It's not always needed, but good for educational purposes. For example

matplotlib/lib/matplotlib/text.py

Lines 1263 to 1264 in 6e95994

raise ValueError("rotation must be 'vertical', 'horizontal' or "
f"a number, not {s}")

Edit, oh maybe you mean value as the parameter name? In that case I would use a more targeted name for the example.

We may also consider adopting a policy to enquote parameter names in single ticks. While there's no universal convention, this is the closest I have found for a pattern, see e.g.


This message helps the user to quickly diagnose and fix their problem.

Remember that many other libraries are implemented on top of Matplotlib, and
therefore ``value`` may not even have been directly passed by the user, but
rather generated in some intermediate layer. In such cases, this kind of
highly explicit messages can be particularly useful, compared to shorter
messages such as ::

raise ValueError("invalid value")
Comment on lines +347 to +349
Copy link
Member

@timhoffm timhoffm Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe inline to de-emphasize?

Suggested change
messages such as ::
raise ValueError("invalid value")
messages such as ``raise ValueError("invalid value")``.

When skimming the section, you primarily notice the heading and the two code blocks, giving the impression that both are relevant. There's no indication that one is a negative example.

image



Internal helpers
~~~~~~~~~~~~~~~~

Matplotlib has a number of helper functions in the ``matplotlib._api`` module
Copy link
Member

@timhoffm timhoffm Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

link

Suggested change
Matplotlib has a number of helper functions in the ``matplotlib._api`` module
Matplotlib has a number of helper functions in the `matplotlib._api` module

named as ``check_*`` which provide nice error messages for common checks
in Matplotlib. Please use these when working on the Matplotlib code base, it
both saves a bunch of duplicate code and ensures our error messages are
consistent across the library. Use them in your own code at your own risk, we
consider them private API and reserve the right change them with no notice on
any release.
Comment on lines +359 to +361
Copy link
Member

@timhoffm timhoffm Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
consistent across the library. Use them in your own code at your own risk, we
consider them private API and reserve the right change them with no notice on
any release.
consistent across the library.

I would leave this out here. It's in an underscored module and the module docstring also explicitly states this.



.. currentmodule:: matplotlib


.. autosummary::
:toctree: _as_gen
:template: autosummary.rst
:nosignatures:

_api.check_isinstance
_api.check_in_list
_api.check_shape
_api.check_getitem
Comment on lines +364 to +375
Copy link
Member

@timhoffm timhoffm Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generates API documentation resulting in duplication

The best would be if one could tell autosummary not to create new API doecumentation and just link to the existing ones, but I think that is not possible (correct me if I'm wrong).

As a workaround, I'd create a manual table or bullet list (with or without copied/hard-coded description).

An alternative with a little more work would be to structure https://matplotlib.org/stable/api/_api_api.html into topic sections and and link to the section on validation/error handling.

Loading