DOC501 should not raise on ImportError

[Matt-Ord]

I'm getting a 'false positive' on DOC501 when my package has an optional dependency on matplotlib

def get_axis_colorbar(axis: Axes) -> Colorbar | None:
    """Get a colorbar attached to the axis."""
    if plt is None:
        msg = "Matplotlib is not installed. Please install it with the 'plot' extra."
        raise ImportError(msg) # Ruff warns here
    for artist in axis.get_children():
        if isinstance(artist, plt.cm.ScalarMappable) and artist.colorbar is not None:
            return artist.colorbar
    return None

Using ruff 0.8. It might be nice to be able to exclude ImportError from this rule, as it is not an error which a user would be 'expected' to see during runtime.

[tjkuson]

Hi, not a maintainer, just someone interested in the DOC rules! Would you mind explaining why ImportError is special? My intuition (perhaps misguided) is that I would quite like a function that specifically raises ImportError to document that behaviour. To me, the fact that it's an exception type that's raised explicitly rarely seems to make it all the more worth documenting.

[Matt-Ord]

The thought was that it was not really a 'runtime error' as I would expect to document to a user but an install time error. If it is just the result of a user not properly installing downstream dependencies, I would say it would probably be better just to document the required dependency but not the error in this case.

[MichaReiser]

The thought was that it was not really a 'runtime error' as I would expect to document to a user but an install time error.

It's a nice touch that you hint users towards installing Matplotlib with the plot extra. But I do agree with @tjkuson that I would expect the exception to be documented if I opted into DOC501 because a missing extra otherwise manifests as an AttributeError and not necessarily as an ImportError. The difference might surprise someone calling get_axis_colorbar as a library-function.

In your situation, I would suppress the violation with # noqa: DOC501 or consider doing the check earlier, maybe in a main function or when initializing the library to avoid repeating it over multiple functions. Unless this function belongs to a subset of functions that are only sometimes supported (e.g. when installed with an extra), then raising inside the specific library function does make sense, but I would then also expect this to be documented.

[Matt-Ord]

Thanks for taking the time to respond - this is indeed part of a subset of functions, rather than a core feature which is the reason behind the 'plot' extra. I'll close the issue in this case.