Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
timothydijamco opened this issue Dec 23, 2020 · 3 comments
Closed

Comments

@timothydijamco
Copy link

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()
  • There is a global variable foo
  • Its value is an object whose __eq__ behavior is strict, and may raise an error

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

  • Python version: 3.9.0
  • Sphinx version: 3.4.0
  • Sphinx extensions: sphinx.ext.autosummary
@timothydijamco
Copy link
Author

timothydijamco commented Dec 23, 2020

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 tk0miya added this to the 3.4.1 milestone Dec 24, 2020
tk0miya added a commit to tk0miya/sphinx that referenced this issue Dec 24, 2020
…eq__`` method

It should be compared by `is` keyword instead.
@tk0miya
Copy link
Member

tk0miya commented Dec 24, 2020

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

tk0miya added a commit that referenced this issue Dec 24, 2020
Fix #8583: autodoc: Unnecessary object comparision via ``__eq__`` method
@tk0miya tk0miya closed this as completed Dec 24, 2020
@timothydijamco
Copy link
Author

thanks @tk0miya!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants