sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.39k stars 2.09k forks source link

BUG: Autosummary may fail on global variables with overriden __eq__ #8583

Closed timothydijamco closed 3 years ago

timothydijamco commented 3 years ago

This is specific to 3.4.0

Autosummary may fail on a global variable whose value is an object with an __eq__ implementation that raises an error.

Example

For example, let's say I have a module that looks like this:

class Foo:
    def __eq__(self, other):
        if not isinstance(other, Foo):
            raise TypeError('Foo objects cannot be compared to non-Foo objects')

foo = Foo()

When running Sphinx, autosummary will fail.

...
reading sources... [100%] generated/mymodule.foo

Exception occurred:
  File "/root/sphinx-eq-bug-repro/mymodule/__init__.py", line 4, in __eq__
    raise TypeError('Foo objects cannot be compared to non-Foo objects')
TypeError: Foo objects cannot be compared to non-Foo objects

(This code example is a little contrived but hopefully it illustrates well enough—it's a very simplified version of our actual use case)

To Reproduce I have a minimal repro here: https://github.com/timothydijamco/sphinx-eq-bug-repro

Running the below will hit the error if using Sphinx 3.4.0 (but not with Sphinx 3.3.1).

$ git clone https://github.com/timothydijamco/sphinx-eq-bug-repro.git
$ cd sphinx-eq-bug-repro
$ cd docs
$ make html

Expected behavior Sphinx should finish building successfully (it builds OK when using 3.3.1).

Environment info

timothydijamco commented 3 years ago

I believe the cause is the use of == in UninitializedGlobalVariableMixin.should_suppress_value_header (introduced in #8500):

    def should_suppress_value_header(self) -> bool:
        return (self.object == UNINITIALIZED_ATTR or
                super().should_suppress_value_header())

I think == can be changed to is, which is how UninitializedInstanceAttributeMixin.should_suppress_value_header is implemented:

    def should_suppress_value_header(self) -> bool:
        return (self.object is UNINITIALIZED_ATTR or
                super().should_suppress_value_header())
tk0miya commented 3 years ago

Thank you for reporting. Indeed, this must be a bug!

timothydijamco commented 3 years ago

thanks @tk0miya!