bpo-31315: Fix an assertion failure in imp.create_dynamic(), when spec.name is not a string by orenmn · Pull Request #3257 · python/cpython (original) (raw)

Choose a reason for hiding this comment

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

Let me be clear that my recommendation here is not critical. Rather it is a reflection of my belief that little suboptimal things add up over time to become pain points. Docstrings (including in test code) are one of those things we cut corners on sometimes.

While there is precedent (unfortunately) for docstrings formatted this way, I'd rather this docstring were a bit more idiomatic. It's better to take a little time now to strengthen a healthier precedent. This is a case where a little bit of extra time spent adds up over time to make a positive difference. I don't mean to be pedantic here about a relatively insignificant style point or to waste anyone's time, but instead want to encourage a more thoughtful approach with regard to the little things. :) With that out of the way...

I'd expect a docstring here like one of the following (and including some shortening):

# one line
"""Decorator to skip a test if not running under CPython or lacking imp.create_dynamic()."""

or

# one line with separate quotation mark lines
"""
Decorator to skip a test if not running under CPython or lacking imp.create_dynamic().
"""

or

# split into multiple lines (my preference)
"""A decorator to skip tests that rely on imp.create_dynamic().

Not all Python implementations necessarily provide imp.create_dynamic().
This decorators causes decorated tests to be skipped through the normal
unittest mechanism.
"""