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

annotate class variables using typing.ClassVar #12067

Open
danieleades opened this issue Mar 12, 2024 · 1 comment
Open

annotate class variables using typing.ClassVar #12067

danieleades opened this issue Mar 12, 2024 · 1 comment
Labels

Comments

@danieleades
Copy link
Contributor

Explicitly annotating class variables using typing.ClassVar creates greater type safety.
Furthermore, migrating to types-docutils will require this since the upstream types use explicit class variable annotation and mypy will return errors which sphinx subclasses overwrite these without explicit ClassVar annotations.

This is already causing downstream errors for users who are using Sphinx with types-docutils (see python/typeshed#11550 (comment))

@danieleades danieleades added the type:proposal a feature suggestion label Mar 12, 2024
@danieleades
Copy link
Contributor Author

danieleades commented Mar 12, 2024

note that sphinx (or docutils?) uses some weird patterns that make this non-trivial.
For example:

class SphinxI18nReader(SphinxBaseReader):
    """
    A document reader for i18n.

    This returns the source line number of original text as current source line number
    to let users know where the error happened.
    Because the translated texts are partial and they don't have correct line numbers.
    """

    def setup(self, app: Sphinx) -> None:
        super().setup(app)

        self.transforms = self.transforms + app.registry.get_transforms()
        unused = [PreserveTranslatableMessages, Locale, RemoveTranslatableInline,
                  AutoIndexUpgrader, SphinxDomains, DoctreeReadEvent,
                  UIDTransform]
        for transform in unused:
            if transform in self.transforms:
                self.transforms.remove(transform)

SphinxI18nReader subclasses SphinxBaseReader, which defines a class variable transforms

so in SphinxI18nReader we're overwriting a class variable with an instance variable. This is definitely an antipattern, and mypy will rightly complain about it.

So this won't just be a simple case of sprinkling ClassVar annotations around.
Of course once this is done, the ClassVar annotations will prevent these antipatterns in future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants