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

tacaswell · 2023-02-22T19:59:22Z

PR Summary

After leaving a comment to make error messages verbose I felt inspired to write it up for the docs...

Adding generated API docs for the private API might be a bit controversial, but I think on net it is the right thing to do so that we if we do remove them we can be sure that the docs do not still advocate for their use and saying "there are some check_* functions" felt too vague).

ksunden · 2023-02-22T20:06:57Z

doc/devel/contributing.rst

+In cases like this it may be possible to get into a debugging and from reading
+the stack trace sort out right local variable to check is to start to sort out
+what went wrong.


not sure what is meant by "to get into a debugging", and not sure it's made better in context by saying "to get into a debugger", which was my first guess.

did mean "debugger".

story645 · 2023-02-22T20:09:23Z

doc/devel/contributing.rst

+While this does tell the user what variable and value Matplotlib rejected, it still
+does not tell them why.
+
+An example of a **best** error message ::


Throwing your advice back to you, what about starting with the recommended best practice so that it won't get lost in skimming/fatigue? Then if it's still necessary, discuss unpack why the bad is bad - but honestly I'm not sure it's necessary given I think the paragraph under here does a good job of explaining why folks should do it this way.

Agree with this - the first draft is pretty verbose....

story645 · 2023-02-22T20:57:55Z

doc/devel/contributing.rst

+Which tells the user what input in broken, what was passed in, and the reason
+why the value was invalid.   All of this together gives the user the best chance


This is a personal preference, but I like this sort of info broken out into a list:

We recommend writing an error message that states:

which parameter is raising the error

what value was passed

why the value is incorrect

QuLogic · 2023-02-23T04:14:40Z

doc/devel/contributing.rst

+In cases like this it may be possible to get into an interactive debugger and
+from reading the stack trace sort out right local variable to check is to start
+to sort out what went wrong.  However, without access to the running process the user


After the and, this sentence kind of gets a bit muddled.

QuLogic · 2023-02-23T04:16:18Z

doc/devel/contributing.rst

+Internal helpers
+~~~~~~~~~~~~~~~~
+
+Matplotlib has number of helper functions in the ``matplotlib._api`` module


Suggested change

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

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

oscargus · 2023-02-23T14:29:21Z

doc/devel/contributing.rst

+~~~~~~~~~~~~~~~~
+
+Matplotlib has number of helper functions in the ``matplotlib._api`` module
+named as ``check_*`` which provide nice error messages for very common checks


Suggested change

named as ``check_*`` which provide nice error messages for very common checks

named as ``check_*`` which provide nice error messages for common checks

very seems superfluous?

oscargus · 2023-02-23T14:30:47Z

doc/devel/contributing.rst

+
+This message tells the user what (local) variable Matplotlib checked was and
+what value was passed in which can help users rapidly find bugs in their own
+code.  For example if they expected to have passed in an integer but the error


Suggested change

code. For example if they expected to have passed in an integer but the error

code. For example, if they expected to have passed in an integer but the error

oscargus · 2023-02-23T14:31:06Z

doc/devel/contributing.rst

+This message tells the user what (local) variable Matplotlib checked was and
+what value was passed in which can help users rapidly find bugs in their own
+code.  For example if they expected to have passed in an integer but the error
+messages reports the string ``'aardvark'``, they may know where in their code


Suggested change

messages reports the string ``'aardvark'``, they may know where in their code

message reports the string ``'aardvark'``, they may know where in their code

oscargus · 2023-02-23T14:32:22Z

I'm missing info about dropping punctuation characters and if an error message should be in Sentence case or nor.

anntzer · 2023-02-23T16:31:20Z

Highly nitpicky, but my personal(?) view is to do what cpython does:

>>> object()()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'object' object is not callable
>>> object().bar
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'object' object has no attribute 'bar'
>>> getattr(object(), 42)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: attribute name must be string, not 'int'

i.e. no leading capital and no trailing punctuation.
However, if the error message spans multiple sentences, then all sentences should be capitalized and punctuated correctly. I try to avoid such cases, e.g. I would reword the error message of check_shape (currently 'arg' must be 2D with shape (M, 2). Your input has shape (3, 3).) to 'arg' must be 2D with shape (M, 2); your input has shape (3, 3)

anntzer · 2023-02-23T16:45:23Z

I agree with others that this seems pretty verbose (though a good idea to document); a first try at editing (while keeping most of your wording) gives me

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")

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")

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com> Co-authored-by: Antony Lee <anntzer.lee@gmail.com> Co-authored-by: Oscar Gustafsson <oscar.gustafsson@gmail.com>

tacaswell · 2023-03-31T20:24:03Z

I took @anntzer 's wording whole sale and applied the other changes.

QuLogic · 2023-04-06T01:18:40Z

doc/devel/contributing.rst

+--------------
+
+
+Error messages should err on the side of being verbose, descriptive, and


Suggested change

--------------

Error messages should err on the side of being verbose, descriptive, and

--------------

Error messages should err on the side of being verbose, descriptive, and

QuLogic · 2023-04-06T01:19:48Z

doc/devel/contributing.rst

+.. currentmodule:: matplotlib
+
+
+.. autosummary::
+   :toctree: _as_gen
+   :template: autosummary.rst
+   :nosignatures:
+
+   _api.check_isinstance
+   _api.check_in_list
+   _api.check_shape
+   _api.check_getitem


These appear to already have been documented somewhere since the build is failing.

tacaswell added the Documentation label Feb 22, 2023

tacaswell added this to the v3.8.0 milestone Feb 22, 2023

tacaswell mentioned this pull request Feb 22, 2023

Bugfix for loc legend validation #25281

Merged

6 tasks

ksunden reviewed Feb 22, 2023

View reviewed changes

story645 reviewed Feb 22, 2023

View reviewed changes

QuLogic reviewed Feb 23, 2023

View reviewed changes

oscargus reviewed Feb 23, 2023

View reviewed changes

jklymak added the status: needs revision label Feb 24, 2023

DOC: add section about how to write good error messages

4ded56e

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com> Co-authored-by: Antony Lee <anntzer.lee@gmail.com> Co-authored-by: Oscar Gustafsson <oscar.gustafsson@gmail.com>

tacaswell force-pushed the doc/verbose_errors branch from 9a1a219 to 4ded56e Compare March 31, 2023 20:23

tacaswell added status: needs review and removed status: needs revision labels Mar 31, 2023

QuLogic reviewed Apr 6, 2023

View reviewed changes

github-actions bot added the status: needs rebase label May 3, 2023

ksunden modified the milestones: v3.8.0, v3.8-doc Aug 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

tacaswell commented Feb 22, 2023

ksunden Feb 22, 2023

tacaswell Feb 22, 2023

story645 Feb 22, 2023

jklymak Feb 22, 2023

story645 Feb 22, 2023

QuLogic Feb 23, 2023

QuLogic Feb 23, 2023

oscargus Feb 23, 2023

oscargus Feb 23, 2023

oscargus Feb 23, 2023

oscargus commented Feb 23, 2023

anntzer commented Feb 23, 2023

anntzer commented Feb 23, 2023 •

edited

Loading

tacaswell commented Mar 31, 2023

QuLogic Apr 6, 2023

QuLogic Apr 6, 2023

		Which tells the user what input in broken, what was passed in, and the reason
		why the value was invalid. All of this together gives the user the best chance

	Matplotlib has 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 very common checks
	named as ``check_*`` which provide nice error messages for common checks

	code. For example if they expected to have passed in an integer but the error
	code. For example, if they expected to have passed in an integer but the error

	messages reports the string ``'aardvark'``, they may know where in their code
	message reports the string ``'aardvark'``, they may know where in their code

		--------------


		Error messages should err on the side of being verbose, descriptive, and

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

Are you sure you want to change the base?

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

Conversation

tacaswell commented Feb 22, 2023

PR Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscargus commented Feb 23, 2023

anntzer commented Feb 23, 2023

anntzer commented Feb 23, 2023 • edited Loading

tacaswell commented Mar 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anntzer commented Feb 23, 2023 •

edited

Loading