search

sphinx-contrib / spelling

A spelling checker for Sphinx-based documentation
https://sphinxcontrib-spelling.readthedocs.io/en/latest/
BSD 2-Clause "Simplified" License
82 stars 48 forks source link

filters: do not use imp in ImportableModuleFilter #137

Closed dhellmann closed 2 years ago

dhellmann commented 2 years ago

The imp module is deprecated, so replace it with calls to importlib.

Addresses #124

dhellmann commented 2 years ago

/cc @kloczek

adamchainz commented 2 years ago

What about the concerns in the original revert in #105 by @jdufresne ? find_spec will import all parent modules which may have side effects.

I had a look whilst writing #129 and found that Python still doesn't seem to provide a real alternative to imp.

dhellmann commented 2 years ago

What about the concerns in the original revert in #105 by @jdufresne ? find_spec will import all parent modules which may have side effects.

Bah, I forgot about that. Thank you for reminding me.

I had a look whilst writing #129 and found that Python still doesn't seem to provide a real alternative to imp.

Well, let's see what happens with the current implementation. It's true there could be side-effects, but I wonder how serious those are, how frequently they occur, and whether they can be mitigated in other ways.

adamchainz commented 2 years ago

Well, let's see what happens with the current implementation. It's true there could be side-effects, but I wonder how serious those are, how frequently they occur, and whether they can be mitigated in other ways.

This is true.

One suggestion would be to disable this filter by default. It would probably not be too disruptive.

I disabled it for my projects, as I noted in this post just now about #129: https://adamj.eu/tech/2022/01/12/book-driven-development-from-boost-your-django-dx/#contribution-sphinxcontrib-spelling-optimization