Repository navigation
TYP: add type hints for explainers.tf_utils - #4445
SarthakB11 wants to merge 2 commits into
Conversation
…tils Add type annotations to all functions in tf_utils.py. Use type: ignore comments for the lazy tf module import pattern and union-attr access on the optional tf global. Remove module from mypy check_untyped_defs exclusion list. Part of shap#3730
CloseChoice
left a comment
CloseChoice
left a comment
There was a problem hiding this comment.
Thanks for the PR but I don't see a lot of value in here. Just ignoring all warnings and using Any does not really help, it just silences mypy to a similar point as to disable it entirely.
Therefore I would suggest to use typing.Protocol to build a protocol for model, e.g.
class TFModel(typing.Protocol):
# define all necessary methods in hereAddresses @CloseChoice review on shap#4445: instead of using bare Any for model parameters, define a structural protocol that captures the attributes this module actually accesses (inputs, outputs, layers). The tuple branch is moved before the keras-model branch so mypy can narrow the union via isinstance(model, tuple). Behavior is preserved: tuples never matched the keras checks before, so reordering does not change runtime semantics. Session and graph parameters/returns kept as Any: those values are opaque pass-throughs in this file (no method calls on them) and TensorFlow types are not worth a TYPE_CHECKING import for parameters that are never accessed structurally.
|
Pushed a follow-up: defined a Left |
|
Rebased onto latest main. Ready for another look when you have a chance. |
|
@CloseChoice pushed the TFModel Protocol you asked for and rebased onto latest main. CI green, ready for another look when you have a chance. |
Part of #3730
What: Add type annotations to
shap/explainers/tf_utils.pyand enablecheck_untyped_defsfor this module.Changes:
_import_tf,_get_session,_get_graph,_get_model_inputs,_get_model_outputtfglobal asModuleType | Nonetype: ignore[union-attr]comments where the optionaltfmoduleis accessed after the
_import_tf()guard (mypy can't infer the narrowingacross the global assignment)
shap.explainers.tf_utilsfrom the mypy exclusion list inpyproject.tomlVerification:
mypy --check-untyped-defspasses with zero errors on thismodule.
ruff checkpasses. Runtime imports verified. No TF-specific testsaffected (TF is an optional dependency).
AI Disclosure
AI tools (Claude) were used for initial investigation and annotation scaffolding.
All code was reviewed and verified manually.