Skip to content

Conversation

@rhshadrach
Copy link
Member

@rhshadrach rhshadrach commented Nov 4, 2025

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

It appears that if the __module__ value of the class does not agree with the __module__ value of the methods, then the doctests do not run.

Still verifying if this is correct and investigating the best way to handle it.

Edit: Indeed, CI is showing many wrong docstrings due to this.

There are two approaches I am considering here:

  1. Use an env variable similar to what this PR is currently doing to not set the __module__ values when we run the doctests. The downside is that we'd need to not have the __module__ value appear in any of the example's output because e.g. pandas.Series would appear as pandas.core.series.Series.
  2. Set the __module__ on all class methods via the set_module decorator using inspect (automatically iterating through class methods and modifying the __module__ attribute). This would increase import time (not sure by how much) and it wouldn't work on cdef classes.

@jbrockmendel
Copy link
Member

Yikes. I like the env variable approach more than then set_module-on-all-classmethods approach. Is there a "hope for a fix upstream" option?

@rhshadrach
Copy link
Member Author

rhshadrach commented Nov 6, 2025

Is there a "hope for a fix upstream" option?

It seems feasible. The current behavior is due to

https://github.com/python/cpython/blob/9bf5100037f661f3a369d3ee539bec06f063b650/Lib/doctest.py#L1069-L1072

which was added in the commit python/cpython@8485b56. I don't think I can find any discussion due to the age here, so not clear if the added line was thought through. Removing the _from_module check would resolve.

A fix also seems possible in pytest - it overrides DoctestModule._from_module in some cases, and it seems like adding

if inspect.isfunction(object) and "." in object.__qualname__:
    return True

is a reliable way to ignore checking the module of attributes on a class. The class would only be recursed if it was already found to have the right __module__, so it should be safe to always return True here.

Not sure if either CPython or pytest would be on board with an upstream fix - one possible counter is that we shouldn't have a class's __module__ disagree with that of the methods of the class in the first place. But if one is in such a state, it's not clear to me why you would ever want to skip the methods defined in a class when that class is in the right module.

Edit:

Super hacky, but we can also do this ourselves.

def pytest_sessionstart(session):
    import doctest, inspect

    orig = doctest.DocTestFinder._from_module

    def _from_module(self, module, object):
        if inspect.isfunction(object) and "." in object.__qualname__:
            return True
        return orig(self, module, object)

    doctest.DocTestFinder._from_module = _from_module

@mroeschke
Copy link
Member

It's unfortunate, but I'm OK with your pytest_sessionstart solution. I suppose this was an unforeseen side effect of customizing our __module__ attribute.

It would be interesting to survey other projects that override __module__ and how they are running their doctests, if any.

@rhshadrach rhshadrach mentioned this pull request Nov 6, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants