sphinx icon indicating copy to clipboard operation
sphinx copied to clipboard

make examples 'type safe'

Open danieleades opened this issue 3 years ago • 2 comments

This project is making admirable progress towards a fully type-annotated code base.

The tutorials/examples are not type annotated, and in fact fail a mypy run. We should set a good example and show users how to generate sphinx extensions that are fully typed.

This may involve refactoring this codebase to make it easier to create type-safe extensions, though I expect to be able to make a good go of it without any breaking changes.

danieleades avatar Aug 01 '22 08:08 danieleades

I would just like to mention that adding type annotations to the examples also has a cost.

For Python users (like me) who have no experience with type annotations, they make the examples more bulky and confusing.

This may involve refactoring this codebase to make it easier to create type-safe extensions

I agree that is would be good to make it easy for people to write type-safe extensions if they want that, but we should also try to not make it harder for people who don't want to use type annotations in their extensions.

So maybe it would be good to have one example (ideally the most advanced one) with type annotations, but leave the simpler examples without type annotations?

mgeier avatar Aug 05 '22 07:08 mgeier

Thanks for the perspective @mgeier! I have a few comments to make on that

  • Type annotations are pretty standard in Python these days. Also creating a Sphinx extension is non-trivial task, so I would typically expect it to be performed by a user who was familiar with type annotations. That said, I take your point about this being intimidating to developers who are not used to seeing them.
  • The problem with the current examples as they stand is not only that they lack type annotations, but that there are type errors in them. Adding type annotations allows a linter such as mypy to detect the errors that are already there. These examples should be refactored to be to type safe, even if the annotations are stripped out before publishing. For example, in the 'todo' example, we have the following line-
filename = env.doc2path(todo_info['docname'], base=None)

'base' is typed in the doc2path method as a bool, but in this example None is used. It passes because None is falsy, but it's relying on unspecified behaviour that may change if the method is refactored. Type-checking would catch this.

  • whether or not type annotations are used in the examples, Sphinx's APIs are not hugely friendly to downstream users who do want to create type-safe extensions. For example, storing/retrieving state directly on the BuildEnvironment object using getattr will confound any static analysis. The use of untyped dictionaries creates a similar challenge. There are small things that could be done to improve this, such as using a TypedDict to type the return value of the setup method. This would have benefits for downstream users, whether the examples used annotations or not.

perhaps you could review the specific changes made in #10740 and let me know which changes you think add value, and which make it more difficult to follow?

danieleades avatar Aug 05 '22 08:08 danieleades