jupyterlite / jupyterlite-sphinx

Sphinx extension using JupyterLite to render Notebooks
https://jupyterlite-sphinx.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
68 stars 21 forks source link

Minor refactor + typing info #155

Closed Carreau closed 7 months ago

Carreau commented 8 months ago

This adds mypy configuration to ignore voici (untyped), and slightly refactor to pass some kwargs explicitely.

It also pleases mypy.

Carreau commented 7 months ago

Thanks for the review.

I tend to avoid unnecessary from __future__ import annotations, I found that unless necessary they tend to hide bugs in older versions of Python, in particular at runtime if you use get_type_hints.

More particularly, from __future__ import annotations make annotation lazy and allow you to write things that are invalid – typically you can start using list[...] in types, but this is invalid on earlier python.

As for isort, no objections, but I thought the precommit would take care of it ? If not we can just add it.

agriyakhetarpal commented 7 months ago

Thanks for the info, TIL that future-annotations can get us invalid types on earlier Python versions like that. I guess we would have to enable the isort rule before pre-commit starts to apply it across the codebase, but I suppose that's something to do in a separate PR.

Carreau commented 7 months ago

No problem, thanks for the review and merge.