Open Corwinpro opened 1 year ago
Need to decide whether this is worthwhile or not.
Pros
mypy --strict
work out of the box.Cons
__init__.py
reexports must be specified twice, once as a string)--no-implicit-reexport
, including just for __init__.py
filesrequires lots of code duplication (1000s of init.py reexports must be specified twice, once as a string)
This is not true - this behaviour is not code duplication, this is being explicit on what trieste
wants to reexport.
easy to work around by disabling --no-implicit-reexport, including just for init.py files
Where would that flag go? Not sure how a user of trieste
would solve that for trieste
's init.
no obvious code quality benefit (what sort of error are we actually protecting against here?)
See this:
error: Class cannot subclass "AcquisitionFunctionClass" (has type "Any") [misc]
which originates from the same issue.
We'll probably want to fix this in gpflux before fixing it in trieste. AFAICT gpflow already uses __all__
.
Also we'd probably want to generate and verify __all__
programmatically as there are 100s or 1000s of API definitions reexported from the modules they're implemented in. Adding no_implicit_reexport = true
to mypy.ini will catch the ones we use internally but people will inevitably sometimes forget to update __all__
when adding new definitions and fixing this problem by making the code less maintainable is not obviously a net gain.
See this:
Running
mypy --strict
on this yields:Changing the import to
solves the problem, but then the
trieste.acquisition.__init__
is pointless.How to solve
Add
__all__ = ["AcquisitionFunctionClass", ...]
(and everything that needs to be reimported) to thetrieste.acquisition.__init__
.