captum
captum copied to clipboard
Fix multiple Sphinx warnings & docstrings
The warning messages take up a lot of space on the console log, and it was really simple to resolve them.
The common.rst file was also incorrectly pointing to the wrong path and some of functions were renamed since it was created, so no docs were being generated for that page. InputBaselineXGradient
was also removed from the public rst api docs, as it's not supposed to be public.
In addition to these easy doc warning fixes, I found a module that was listed on on the API docs site, but there's not public path to use it and no tests were ever written for it. I made an issue post for it here: https://github.com/pytorch/captum/issues/989
I fixed some docstring issues like lack of consistent uppercase for any
and callable
, spacing, random type / case mistakes I came across, etc...
I also fixed some paths.
Issues with upgrading to later versions of Sphinx were also resolved.
Currently Sphinx gives the following warnings / errors for the master branch:
/content/captum/sphinx/source/base_classes.rst:2: WARNING: Title underline too short.
Base Classes
==========
/content/captum/sphinx/source/base_classes.rst:29: WARNING: Title underline too short.
Perturbation Attribution
^^^^^^^^^^^^^^^^^^^^^
/content/captum/sphinx/source/base_classes.rst:29: WARNING: Title underline too short.
Perturbation Attribution
^^^^^^^^^^^^^^^^^^^^^
WARNING: autodoc: failed to import function 'validate_input' from module 'captum.attr._utils.common'; the following exception was raised:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 448, in safe_getattr
return getattr(obj, name, *defargs)
AttributeError: module 'captum.attr._utils.common' has no attribute 'validate_input'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/importer.py", line 110, in import_object
obj = attrgetter(obj, mangled_name)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 332, in get_attr
return autodoc_attrgetter(self.env.app, obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 2780, in autodoc_attrgetter
return safe_getattr(obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 464, in safe_getattr
raise AttributeError(name) from exc
AttributeError: validate_input
WARNING: autodoc: failed to import function 'validate_noise_tunnel_type' from module 'captum.attr._utils.common'; the following exception was raised:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 448, in safe_getattr
return getattr(obj, name, *defargs)
AttributeError: module 'captum.attr._utils.common' has no attribute 'validate_noise_tunnel_type'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/importer.py", line 110, in import_object
obj = attrgetter(obj, mangled_name)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 332, in get_attr
return autodoc_attrgetter(self.env.app, obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 2780, in autodoc_attrgetter
return safe_getattr(obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 464, in safe_getattr
raise AttributeError(name) from exc
AttributeError: validate_noise_tunnel_type
WARNING: autodoc: failed to import function 'format_input' from module 'captum.attr._utils.common'; the following exception was raised:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 448, in safe_getattr
return getattr(obj, name, *defargs)
AttributeError: module 'captum.attr._utils.common' has no attribute 'format_input'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/importer.py", line 110, in import_object
obj = attrgetter(obj, mangled_name)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 332, in get_attr
return autodoc_attrgetter(self.env.app, obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 2780, in autodoc_attrgetter
return safe_getattr(obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 464, in safe_getattr
raise AttributeError(name) from exc
AttributeError: format_input
WARNING: autodoc: failed to import function '_format_attributions' from module 'captum.attr._utils.common'; the following exception was raised:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 448, in safe_getattr
return getattr(obj, name, *defargs)
AttributeError: module 'captum.attr._utils.common' has no attribute '_format_attributions'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/importer.py", line 110, in import_object
obj = attrgetter(obj, mangled_name)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 332, in get_attr
return autodoc_attrgetter(self.env.app, obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 2780, in autodoc_attrgetter
return safe_getattr(obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 464, in safe_getattr
raise AttributeError(name) from exc
AttributeError: _format_attributions
WARNING: autodoc: failed to import function 'zeros' from module 'captum.attr._utils.common'; the following exception was raised:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 448, in safe_getattr
return getattr(obj, name, *defargs)
AttributeError: module 'captum.attr._utils.common' has no attribute 'zeros'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/importer.py", line 110, in import_object
obj = attrgetter(obj, mangled_name)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 332, in get_attr
return autodoc_attrgetter(self.env.app, obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 2780, in autodoc_attrgetter
return safe_getattr(obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 464, in safe_getattr
raise AttributeError(name) from exc
AttributeError: zeros
WARNING: autodoc: failed to import function '_run_forward' from module 'captum.attr._utils.common'; the following exception was raised:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 448, in safe_getattr
return getattr(obj, name, *defargs)
AttributeError: module 'captum.attr._utils.common' has no attribute '_run_forward'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/importer.py", line 110, in import_object
obj = attrgetter(obj, mangled_name)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 332, in get_attr
return autodoc_attrgetter(self.env.app, obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 2780, in autodoc_attrgetter
return safe_getattr(obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 464, in safe_getattr
raise AttributeError(name) from exc
AttributeError: _run_forward
/content/captum/sphinx/source/concept.rst:2: WARNING: Title underline too short.
Concept-based Interpretability
======
/content/captum/sphinx/source/concept.rst:12: WARNING: Title underline too short.
ConceptInterpreter
^^^^^^^^^^^^^^^^
/content/captum/sphinx/source/concept.rst:12: WARNING: Title underline too short.
ConceptInterpreter
^^^^^^^^^^^^^^^^
/content/captum/sphinx/source/deconvolution.rst:2: WARNING: Title underline too short.
Deconvolution
=========
/content/captum/captum/attr/_core/deep_lift.py:docstring of captum.attr._core.deep_lift.DeepLiftShap:12: WARNING: Definition list ends without a blank line; unexpected unindent.
/content/captum/sphinx/source/feature_ablation.rst:2: WARNING: Title underline too short.
Feature Ablation
=========
/content/captum/captum/attr/_core/feature_ablation.py:docstring of captum.attr._core.feature_ablation.FeatureAblation.attribute:36: WARNING: Bullet list ends without a blank line; unexpected unindent.
/content/captum/sphinx/source/feature_permutation.rst:2: WARNING: Title underline too short.
Feature Permutation
=========
WARNING: autodoc: failed to import class 'InputBaselineXGradient' from module 'captum.attr'; the following exception was raised:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 448, in safe_getattr
return getattr(obj, name, *defargs)
AttributeError: module 'captum.attr' has no attribute 'InputBaselineXGradient'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/importer.py", line 110, in import_object
obj = attrgetter(obj, mangled_name)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 332, in get_attr
return autodoc_attrgetter(self.env.app, obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/ext/autodoc/__init__.py", line 2780, in autodoc_attrgetter
return safe_getattr(obj, name, *defargs)
File "/usr/local/lib/python3.7/dist-packages/sphinx/util/inspect.py", line 464, in safe_getattr
raise AttributeError(name) from exc
AttributeError: InputBaselineXGradient
/content/captum/sphinx/source/guided_backprop.rst:2: WARNING: Title underline too short.
Guided Backprop
=========
/content/captum/sphinx/source/guided_grad_cam.rst:2: WARNING: Title underline too short.
Guided GradCAM
=========
/content/captum/sphinx/source/influence.rst:2: WARNING: Title underline too short.
Influential Examples
======
/content/captum/sphinx/source/influence.rst:12: WARNING: Title underline too short.
SimilarityInfluence
^^^^^^^^^^^^^^^^^
/content/captum/sphinx/source/influence.rst:12: WARNING: Title underline too short.
SimilarityInfluence
^^^^^^^^^^^^^^^^^
/content/captum/captum/influence/_core/tracincp.py:docstring of captum.influence._core.tracincp.TracInCPBase.influence:61: WARNING: Inline interpreted text or phrase reference start-string without end-string.
/content/captum/captum/influence/_core/tracincp.py:docstring of captum.influence._core.tracincp.TracInCP.influence:61: WARNING: Inline interpreted text or phrase reference start-string without end-string.
/content/captum/captum/influence/_core/tracincp_fast_rand_proj.py:docstring of captum.influence._core.tracincp_fast_rand_proj.TracInCPFast.influence:62: WARNING: Inline interpreted text or phrase reference start-string without end-string.
/content/captum/sphinx/source/influence.rst:38: WARNING: Title underline too short.
TracInCPFastRandProj
^^^^^^^^^^^^^^^^
/content/captum/sphinx/source/influence.rst:38: WARNING: Title underline too short.
TracInCPFastRandProj
^^^^^^^^^^^^^^^^
/content/captum/captum/influence/_core/tracincp_fast_rand_proj.py:docstring of captum.influence._core.tracincp_fast_rand_proj.TracInCPFastRandProj:1: WARNING: Inline literal start-string without end-string.
/content/captum/sphinx/source/input_x_gradient.rst:2: WARNING: Title underline too short.
Input X Gradient
===============
WARNING: autodoc: failed to import class 'api.Batch' from module 'captum.insights'; the following exception was raised:
No module named 'captum.insights.api'
WARNING: autodoc: failed to import class 'api.AttributionVisualizer' from module 'captum.insights'; the following exception was raised:
No module named 'captum.insights.api'
WARNING: autodoc: failed to import class 'features.BaseFeature' from module 'captum.insights'; the following exception was raised:
No module named 'captum.insights.features'
WARNING: autodoc: failed to import class 'features.GeneralFeature' from module 'captum.insights'; the following exception was raised:
No module named 'captum.insights.features'
WARNING: autodoc: failed to import class 'features.TextFeature' from module 'captum.insights'; the following exception was raised:
No module named 'captum.insights.features'
WARNING: autodoc: failed to import class 'features.ImageFeature' from module 'captum.insights'; the following exception was raised:
No module named 'captum.insights.features'
/content/captum/captum/attr/_core/integrated_gradients.py:docstring of captum.attr._core.integrated_gradients.IntegratedGradients.attribute:43: WARNING: Bullet list ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_core/kernel_shap.py:docstring of captum.attr._core.kernel_shap.KernelShap.attribute:66: WARNING: Bullet list ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_core/kernel_shap.py:docstring of captum.attr._core.kernel_shap.KernelShap.kernel_shap_perturb_generator:4: WARNING: Block quote ends without a blank line; unexpected unindent.
/content/captum/sphinx/source/layer.rst:2: WARNING: Title underline too short.
Layer Attribution
======
/content/captum/captum/attr/_core/layer/layer_conductance.py:docstring of captum.attr._core.layer.layer_conductance.LayerConductance.attribute:35: WARNING: Bullet list ends without a blank line; unexpected unindent.
/content/captum/sphinx/source/layer.rst:18: WARNING: Title underline too short.
Internal Influence
^^^^^^^^^^^^^^^^^
/content/captum/sphinx/source/layer.rst:18: WARNING: Title underline too short.
Internal Influence
^^^^^^^^^^^^^^^^^
/content/captum/captum/attr/_core/layer/internal_influence.py:docstring of captum.attr._core.layer.internal_influence.InternalInfluence.attribute:209: WARNING: Inline interpreted text or phrase reference start-string without end-string.
/content/captum/sphinx/source/layer.rst:24: WARNING: Title underline too short.
Layer Gradient X Activation
^^^^^^^^^^^^^^^^^^^^^^^^^
/content/captum/sphinx/source/layer.rst:24: WARNING: Title underline too short.
Layer Gradient X Activation
^^^^^^^^^^^^^^^^^^^^^^^^^
/content/captum/captum/attr/_core/layer/layer_deep_lift.py:docstring of captum.attr._core.layer.layer_deep_lift.LayerDeepLift.attribute:39: WARNING: Bullet list ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_core/layer/layer_deep_lift.py:docstring of captum.attr._core.layer.layer_deep_lift.LayerDeepLiftShap:16: WARNING: Definition list ends without a blank line; unexpected unindent.
/content/captum/sphinx/source/layer.rst:54: WARNING: Title underline too short.
Layer Integrated Gradients
^^^^^^^^^^^^^^^^^^^^^^^^^
/content/captum/sphinx/source/layer.rst:54: WARNING: Title underline too short.
Layer Integrated Gradients
^^^^^^^^^^^^^^^^^^^^^^^^^
/content/captum/captum/attr/_core/layer/layer_integrated_gradients.py:docstring of captum.attr._core.layer.layer_integrated_gradients.LayerIntegratedGradients.attribute:35: WARNING: Unexpected indentation.
/content/captum/captum/attr/_core/layer/layer_integrated_gradients.py:docstring of captum.attr._core.layer.layer_integrated_gradients.LayerIntegratedGradients.attribute:140: WARNING: Unexpected indentation.
/content/captum/captum/attr/_core/layer/layer_integrated_gradients.py:docstring of captum.attr._core.layer.layer_integrated_gradients.LayerIntegratedGradients.attribute:158: WARNING: Block quote ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_core/layer/layer_lrp.py:docstring of captum.attr._core.layer.layer_lrp.LayerLRP.attribute:79: WARNING: Definition list ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_core/layer/layer_lrp.py:docstring of captum.attr._core.layer.layer_lrp.LayerLRP.attribute:93: WARNING: Block quote ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_core/lime.py:docstring of captum.attr._core.lime.LimeBase.attribute:111: WARNING: Inline strong start-string without end-string.
/content/captum/captum/attr/_core/lime.py:docstring of captum.attr._core.lime.Lime.attribute:66: WARNING: Bullet list ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_core/lrp.py:docstring of captum.attr._core.lrp.LRP.attribute:68: WARNING: Unexpected indentation.
/content/captum/captum/attr/_core/lrp.py:docstring of captum.attr._core.lrp.LRP.attribute:80: WARNING: Block quote ends without a blank line; unexpected unindent.
/content/captum/sphinx/source/metrics.rst:2: WARNING: Title underline too short.
Metrics
======
/content/captum/captum/metrics/_core/infidelity.py:docstring of captum.metrics._core.infidelity.infidelity:83: WARNING: Definition list ends without a blank line; unexpected unindent.
/content/captum/captum/metrics/_core/sensitivity.py:docstring of captum.metrics._core.sensitivity.sensitivity_max:112: WARNING: Inline strong start-string without end-string.
/content/captum/sphinx/source/neuron.rst:2: WARNING: Title underline too short.
Neuron Attribution
=======
/content/captum/sphinx/source/neuron.rst:11: WARNING: Title underline too short.
Neuron Integrated Gradients
^^^^^^^^^^^^^^^^^^^^^^^^^^
/content/captum/sphinx/source/neuron.rst:11: WARNING: Title underline too short.
Neuron Integrated Gradients
^^^^^^^^^^^^^^^^^^^^^^^^^^
/content/captum/captum/attr/_core/neuron/neuron_deep_lift.py:docstring of captum.attr._core.neuron.neuron_deep_lift.NeuronDeepLiftShap:16: WARNING: Definition list ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_core/neuron/neuron_feature_ablation.py:docstring of captum.attr._core.neuron.neuron_feature_ablation.NeuronFeatureAblation.attribute:69: WARNING: Bullet list ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_core/noise_tunnel.py:docstring of captum.attr._core.noise_tunnel.NoiseTunnel:18: WARNING: Definition list ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_core/occlusion.py:docstring of captum.attr._core.occlusion.Occlusion.attribute:68: WARNING: Bullet list ends without a blank line; unexpected unindent.
WARNING: autodoc: failed to import module 'pytext' from module 'captum.attr._models'; the following exception was raised:
No module named 'pytext'
WARNING: don't know which module to import for autodocumenting 'PyTextInterpretableEmbedding' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'BaselineGenerator' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
/content/captum/sphinx/source/robust.rst:2: WARNING: Title underline too short.
Robustness
======
/content/captum/sphinx/source/robust.rst:26: WARNING: Title underline too short.
Min Param Perturbation
^^^^^^^^^^^^^^^^
/content/captum/sphinx/source/robust.rst:26: WARNING: Title underline too short.
Min Param Perturbation
^^^^^^^^^^^^^^^^
/content/captum/captum/robust/_core/metrics/min_param_perturbation.py:docstring of captum.robust._core.metrics.min_param_perturbation.MinParamPerturbation:75: WARNING: Inline strong start-string without end-string.
/content/captum/sphinx/source/shapley_value_sampling.rst:2: WARNING: Title underline too short.
Shapley Value Sampling
=========
/content/captum/captum/attr/_core/shapley_value.py:docstring of captum.attr._core.shapley_value.ShapleyValueSampling.attribute:42: WARNING: Bullet list ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_core/shapley_value.py:docstring of captum.attr._core.shapley_value.ShapleyValues.attribute:42: WARNING: Bullet list ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_utils/visualization.py:docstring of captum.attr._utils.visualization.visualize_image_attr:37: WARNING: Enumerated list ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_utils/visualization.py:docstring of captum.attr._utils.visualization.visualize_image_attr:54: WARNING: Enumerated list ends without a blank line; unexpected unindent.
looking for now-outdated files... none found
pickling environment... done
checking consistency... /content/captum/sphinx/source/approximation_methods.rst: WARNING: document isn't included in any toctree
/content/captum/sphinx/source/common.rst: WARNING: document isn't included in any toctree
/content/captum/sphinx/source/pytext.rst: WARNING: document isn't included in any toctree
done
With the changes in this PR, Sphinx now only gives the following warnings instead of the above multi page list:
/content/captum/captum/attr/_core/layer/layer_integrated_gradients.py:docstring of captum.attr._core.layer.layer_integrated_gradients.LayerIntegratedGradients.attribute:147: WARNING: Bullet list ends without a blank line; unexpected unindent.
/content/captum/captum/attr/_core/lime.py:docstring of captum.attr._core.lime.LimeBase.attribute:111: WARNING: Inline strong start-string without end-string.
/content/captum/captum/metrics/_core/sensitivity.py:docstring of captum.metrics._core.sensitivity.sensitivity_max:112: WARNING: Inline strong start-string without end-string.
WARNING: autodoc: failed to import class 'pytext.PyTextInterpretableEmbedding' from module 'captum.attr._models'; the following exception was raised:
No module named 'pytext'
WARNING: autodoc: failed to import class 'pytext.BaselineGenerator' from module 'captum.attr._models'; the following exception was raised:
No module named 'pytext'
/content/captum/captum/robust/_core/metrics/min_param_perturbation.py:docstring of captum.robust._core.metrics.min_param_perturbation.MinParamPerturbation:77: WARNING: Inline strong start-string without end-string.
looking for now-outdated files... none found
pickling environment... done
checking consistency... /content/captum/sphinx/source/pytext.rst: WARNING: document isn't included in any toctree
done
I've come across some issues:
-
Doc variables with the lowercase
any
may be seen as referencing theany()
function rather than the type. So, instead ofany
, we should probably be using the uppercaseAny
. -
The
callable
type also has the same issue asany
, as there is acallable()
function. -
Which example is the correct format for a tuple with a known size
tuple of int
,tuple(int, int)
, ortuple[int, int]
? I've seen conflicting examples in Captum's code. -
Are we supposed to be appending an 's' to the end of types in the doc variables? Ex:
tuple of ints
(with int being plural) instead oftuple of int
as there are conflicting examples in Captum's code. -
Is there specific formatting for dict? Like
dict[int, float]
. -
I assume the format for listing multiple variable type options is
(type1 or type2)
, and(type1, type2, or type3)
?
I've also noticed a major mistake in formatting code snippets in docstrings.
The other Captum modules all use this incorrect format that doesn't show up on the website:
- `code_highlight`: Works for Markdown only:
code_highlight
They should be using this format, which works on the website and Markdown:
- ``code_highlight``: Works for Markdown and reStructuredText (reST):
code_highlight
I'm working on a regex solution to fix this issue Captum wide, but that'll probably be for another PR.
You can also directly link to function / classes based on their name like I show below, but this is not something I can easily automate:
# Other classes and functions in the same file
# Sometimes this format may require the '.' prefix to work
:class:`MyClass`
:func:`myclass.my_func`
:func:`my_func`
# Classes and functions in another file (prefix of '.')
:class:`.MyClass`
:func:`.MyClass.my_func`
:func:`.my_func`
# You can also give exact paths
:class:`captum.module.MyClass`
:func:`captum.module.MyClass.my_func`
:func:`captum.module.my_func`
# Or you can show a different name for the hyperlink link
:func:`my_func_name <captum.module.my_func>`
- callable
Thank you for the suggestion, @ProGamerGov! Regarding any or Any currently I don't see that the website is linking to the right type. Basically it is currently not generating any link on the web site. I see the same issue for Callable. https://captum.ai/api/layer.html Do you see any links generated for Any or any ?
I think that we can use tuple of int
similar to what we do for tensors like tuple of tensors
tuple of int - I think that we don't need to add 's' in the end
I just also looked into pytorch docs and it looks like they just annotate it with dict https://github.com/pytorch/pytorch/blob/master/torch/nn/modules/module.py#L1508 I think that we have been using dict[type1, type2] for most cases.
Do you mean this for tuples: (type1 or type2), and (type1, type2, or type3) ?
@NarineK So for Callable and Any, I am planning on using this function in the conf.py
here to ensure that each type is hyperlinked by intersphinx. Whether or not we use uppercase or lowercase is not important as we can hyperlink either option, but we are linking to the uppercase type hints 'Callable' and 'Any'. The PyTorch library code uses both the upper and lower case versions randomly it seems.
There aren't any links for Callable or Any currently, but the code below will ensure that they are created when generating the Sphinx webpages. (This function hasn't been added to this PR as I was planning to add it with the rest of the Optim module. However, I can add it to the master branch now if you want.)
import re
from typing import List
def autodoc_process_docstring(
app, what: str, name: str, obj, options, lines: List[str]
) -> None:
"""
Modify docstrings before creating html files.
See here for more information:
https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html
"""
for i in range(len(lines)):
# Skip unless line is an parameter doc or a return doc
if not (lines[i].startswith(":type") or lines[i].startswith(":rtype")):
continue
# Change "nn.Module" to "torch.nn.Module" in doc type hints for intersphinx
lines[i] = re.sub(r"\bnn.Module\b", "torch.nn.Module", lines[i])
lines[i] = lines[i].replace("torch.torch.", "torch.")
# Ensure nn.Module and torch.Tensor are hyperlinked
lines[i] = re.sub(r"\btorch.nn.Module\b", ":obj:`torch.nn.Module`", lines[i])
lines[i] = re.sub(r"\btorch.Tensor\b", ":obj:`torch.Tensor`", lines[i])
# Handle Any & Callable types
lines[i] = re.sub(r"\bAny\b", ":obj:`Any <typing.Any>`", lines[i])
lines[i] = re.sub(r"\bany\b", ":obj:`Any <typing.Any>`", lines[i])
lines[i] = re.sub(
r"\bCallable\b", ":obj:`Callable <typing.Callable>`", lines[i]
)
lines[i] = re.sub(
r"\bcallable\b", ":obj:`Callable <typing.Callable>`", lines[i]
)
# Handle list & tuple types
lines[i] = re.sub(r"\blist\b", ":obj:`list`", lines[i])
lines[i] = re.sub(r"\btuple\b", ":obj:`tuple`", lines[i])
# Handle str, bool, & slice types
lines[i] = re.sub(r"\bstr\b", ":obj:`str`", lines[i])
lines[i] = re.sub(r"\bbool\b", ":obj:`bool`", lines[i])
lines[i] = re.sub(r"\bslice\b", ":obj:`slice`", lines[i])
# Handle int & float types
lines[i] = re.sub(r"\bint\b", ":obj:`int`", lines[i])
lines[i] = re.sub(r"\bfloat\b", ":obj:`float`", lines[i])
lines[i] = re.sub(r"\bints\b", ":obj:`ints <int>`", lines[i])
lines[i] = re.sub(r"\bfloats\b", ":obj:`floats <float>`", lines[i])
# Handle tensor types that are using lowercase
# Bolding return types doesn't work with Sphinx hyperlinks
lines[i] = lines[i].replace("*tensors*", "tensors")
lines[i] = lines[i].replace("*tensor*", "tensor")
lines[i] = re.sub(r"\btensor\b", ":class:`tensor <torch.Tensor>`", lines[i])
lines[i] = re.sub(r"\btensors\b", ":class:`tensors <torch.Tensor>`", lines[i])
# Handle None type
lines[i] = re.sub(r"\bNone\b", ":obj:`None`", lines[i])
def setup(app) -> None:
app.connect("autodoc-process-docstring", autodoc_process_docstring)
Do you mean this for tuples: (type1 or type2), and (type1, type2, or type3) ?
I was referring to listing multiple types for a variable in the docs like this:
def func(x, y, z):
"""
Args:
x (type1, type2, or type3): Variable description.
y (type1 or type2): Variable description.
y (tuple of type1 or list of type2): Variable description.
"""
I think I saw a few cases where this was used: (type1 or type2 or type3 or type4)
. Though it's not really that important for this PR.
I ended up going with the uppercase versions of 'Callable' & 'Any' as they are going to be hyperlinked to typing.Callable
& typing.Any
, rather than their lowercase function versions.
For the remaining Sphinx warnings:
-
"WARNING: Inline strong start-string without end-string.": appears to be caused by using *args & **kwargs as variable names in the docstrings, even though it's an acceptable format. This only fixed recently in Sphinx: https://github.com/sphinx-doc/sphinx/pull/10451
-
The rst ones are just webpages that aren't linked to by anything, and they don't show up in the index.
For the Sphinx warnings / errors, I tested using Sphinx v4.3.2 and with Sphinx v5.0.2 (the latest version). So the Sphinx version used by Captum can be pinned to the most recent version without issue.
@NarineK This PR should be ready for merging now, unless there's anything you want changed?
app.connect("autodoc-process-docstring",
Thank you for the prompt reply, @ProGamerGov! Currently the linking for list, tuple , bool works fine. I wonder why are we making replacement in the autodoc_process_docstring
function.
I haven't done this type of replacements before lines[i] = re.sub(r"\bAny\b", ":obj:
Any <typing.Any>", lines[i])
, does it generate the links correctly ? Were you able to generate the website and check ? If you could post couple screenshots that would be great.
I'll ask @aobo-y (cc: @vivekmig) to take a look as well since he has done some fixes on the website.
If we are making changes for any and callable here it would be good to add the linking support in master as well.
...
I don't remember that we have (type1 or type2), and (type1, type2, or type3) definitions in Captum. It's probably very rare. You can use (type1 or type2), and (type1, type2, or type3)
notation in the docs.
@NarineK I threw in the bool linking for future cases that may arise, but I can remove it. The list and tuple linking though does not work for every case at the moment.
You can see examples of failures below from Optim's atlas submodule:
There are also numerous examples of failures on the website API pages for lists & tuples. Example: 'tuple of' hyperlinking fails here for example: https://captum.ai/api/neuron.html#captum.attr.NeuronGradient.attribute, and here: https://captum.ai/api/neuron.html#captum.attr.NeuronGradientShap.attribute
I wonder why are we making replacement in the autodoc_process_docstring function.
It makes the source docs more human readable and easier to write. It also helps ensure that Sphinx correctly hyperlinks each type used, as the automatic detection is not perfect and fails in a number of cases.
For example, here are the docs before the autodoc_process_docstring
function is run:
def func(x, y, z, i):
"""
Args:
x (list of list of int, optional): Variable description.
y (tensor or tuple of tensors): Variable description.
z (Callable, optional): Variable description.
i (Any, optional): Variable description.
"""
And here they are after.
def func(x, y, z, i):
"""
Args:
x (:obj:`list` of :obj:`list` of :obj:`int`, optional): Variable description.
y (:class:`tensor <torch.Tensor>` or :obj:`tuple` of :class:`tensors <torch.Tensor>`): Variable description.
z (:obj:`Callable <typing.Callable>`, optional): Variable description.
i (:obj:`Any <typing.Any>`, optional): Variable description.
"""
It's a lot easier to create the first version of the function, and the code is cleaner as well compared to the second version of the function.
I haven't done this type of replacements before
lines[i] = re.sub(r"\bAny\b", ":obj:
Any <typing.Any>", lines[i])
, does it generate the links correctly ? Where you able to generate the website and check ? If you could post couple screenshots that would be great.
Yeah, it works correctly and I've been testing the changes I make by generating webpages.
Example of Any used in the common additional_forward_args
argument :
And an example of Callable in FGSM:
Note in the example images that I was only running the Sphinx build command via the provided script like this:
%cd captum/sphinx
!make html
%cd ..
%cd ..
So, they are missing the final coloring tweaks that the subsequent website building scripts perform. As a result I think that the hyperlinked types in the docs can show up as bold.
If we are making changes for any and callable here it would be good to add the linking support in master as well.
I'll add the function to conf.py
in this PR then! I also removed the bool replacement part.
https://captum.ai/api/neuron.html#captum.attr.NeuronGradient.attribute
@ProGamerGov, I see that in many cases we wrote tuple of ints
and int didn't get highlighted here. We can make it tuple of int
and it might get highlighted but it will be a grammatical error.
In your screenshot I see that for example you have torch.Tensor
annotation but we usually used tensor that's another inconsistency.
In which stage of development is this documentation being generated ? I find :obj:
not very human readable. I hope it's not in the source code.
def func(x, y, z, i):
"""
Args:
x (:obj:`list` of :obj:`list` of :obj:`int`, optional): Variable description.
y (:class:`tensor <torch.Tensor>` or :obj:`tuple` of :class:`tensors <torch.Tensor>`): Variable description.
z (:obj:`Callable <typing.Callable>`, optional): Variable description.
i (:obj:`Any <typing.Any>`, optional): Variable description.
"""
I think that we need to check out this PR and test on our machines before we approve it. I'll talk to @aobo-y about it tomorrow. Either Oliver or I can do it.
@NarineK I've already changed the plural versions of the types like ints
to int
, and floats
to floats
. It's grammatically incorrect, but its the Google style Python docstring format for Sphinx that Captum is using: https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html
From the Google style Python docstring examples in the link:
param3 (:obj:`list` of :obj:`str`): Description of `param3`.
Sphinx will often only highlight the first type though when doing: type1 of type2
, and that's why I am using the function in the conf.py
to explicitly tell Sphinx what to highlight.
For the torch.Tensor
stuff, yeah I'm going to have to change that in my Optim code, unless we want to have two possible ways of doing it. I was thinking that it looked better for the Optim module to use torch.Tensor
, but I guess it is inconsistent and might be a bit redundant as its PyTorch code (and thus PyTorch's torch.Tensor
type). Though maybe it might an aesthetic choice for modules, as long as they stick to the same format throughout.
The :obj:
stuff is done in the conf.py
on the internal copies of the Python files that Sphinx uses to generate the docs. I did it this way so that the Python source code wasn't filled with messily filled with :obj:
in the docstring variable types. The Sphinx viewcode extension's source code is also free of :obj:
as well, as the modification is used only for creating the html code.
Unfortunately, as you may have noticed, PyTorch ecosystem, or even the core torch lib itself, does not have a standardized way of writing docstring. For example, the docs of cnn has similar typing like us while rnn and transformer do not even have typing. torchvision has types in function signature instead and use parameter section for tensor shape, while torchrec directly uses the latest Python typing in the doc. Personally, I feel using function signature with native Python typing and default value is the most clear way to users and it requires little efforts from developers, but it needs too many changes across the whole captum code base. We should stick to our current docstring "convention" for now and see if the community would like to unify the doc in future.
For our docstring convention, we will standardize it and add it into our developer guide. There should be a unified way of writing to give both developer and source code reader a unified experience. I support most of the efforts in this pr that tries to unify the writing, specifically:
- No plural,
tuple of ints
->tuple of int
-
any
->Any
-
callable
->Callable
-
string
->str
,boolean
tobool
,dictionary
->dict
- Move
optional
to the end
However, for the same reason, I am against let autodoc_process_docstring
support multiple ways of writing same type, like nn.Module
and torch.nn.Module
. Developer is responsible to follow the convention (after we have one of course).
There are some changes that should also be delayed from this PR, at least after we finalize our docstring convention:
-
tensor
; Most of the PyTorch's docs use eitherTensor
ortorch.Tensor
, which are both capitalized at least. It is likely we will agree on one of these 2, so we should not change any existing cases totensor
-
List[xx]
->list of xx
; I know we have mixed use cases while PyTorch uses the 2nd one. But the 2nd format is really limited. It become very messy for nested lists, e.g.,list of list of Concept
and it cannot be extended todict
. Libs liketorchrec
andtorchaudio
are all using the 1st format. So we do not need to unify them for now before we make a final choice.
cc @NarineK
@aobo-y I would like to add that having a proper standard across Captum will make it easier to adopt any future standards. Stuff like regex and other text replacement tools can then be used to match and replace the desired patterns. So, in another PR for example we can convert all mentions of tensor
& tensors
to the Capitalized standard we decide on.
I got the boolean
-> bool
and string
-> str
changes implemented into the PyTorch master branch: https://github.com/pytorch/pytorch/pull/82410
I also got the plural ints
-> int
changes implemented into the PyTorch master branch: https://github.com/pytorch/pytorch/pull/82474
The callable
-> Callable
change was implemented as well: https://github.com/pytorch/pytorch/pull/82487
I have built the website and test this change. Unfortunately, autodoc_preserve_defaults
breaks our style. The style issue may be because :obj is for code piece so it inserts an extra html code tag
I would suggest revert conf.py
, as beautify links have nth to do with diff. We should focus on getting the sphinx errors fixed.
@aobo-y Okay, I've removed it for this PR! Hopefully though there's some way around that issue that we can tackle in a future PR as its useful for simplifying function signatures.
I'm seeing the additional tag used for the hyperlinks, but I am not seeing any wrong with the site. I don't see any signs that the site style is broken with or without autodoc_preserve_defaults
.
Can you share some examples of what you mean by breaking the style? Is it the lack of italics? If it's the italics, I can remove the replacement function and then readd it at a later date once I've figured out how to fix it.
@aobo-y So, after tracing back the italics style issue, I found the source of Sphinx not properly handling type1 of type2
cases in docstrings. I then worked with the Sphinx developers to resolve the issue in: https://github.com/sphinx-doc/sphinx/pull/10738
So, by updating from Sphinx v4.3.2 to Sphinx v5.x, the hyperlink issue that I created the autodoc_process_docstring
function to solve, would be resolved without any changes to Captum.
Therefore, I've removed the autodoc_process_docstring
in favor of updating the Sphinx version to v5.2.0 when it is released.
I've tested and compared the diffs between Sphinx v4.3.2 and v5.1.1, for the generated webpages. There is only one minor thing that needs to be addressed before we update the Sphinx version.
Previously the "Quick Search" option on each page used this HTML code for Sphinx v4.3.2:
</div>
</div>
<script>$('#searchbox').show(0);</script>
</div>
</div>
In Sphinx v5.x, the "Quick Search" option on each page uses this HTML code:
</div>
</div>
<script>document.getElementById('searchbox').style.display = "block"</script>
</div>
</div>
Currently pip's version matchers / installer decides on installing Sphinx v4.3.2 due to the required sphinx-autodoc-typehints
package in setup.py
. Without specifying a minimum version for sphinx-autodoc-typehints
or sphinx
, pip ends up installing an old version of Sphinx. The Sphinx dependencies in Captum aren't currently pinned by version, but they are stuck at the same version by pip's inability to match the right versions.
I haven't updated the Sphinx version in this PR.
Edit:
I think the new "Quick Search" code essentially works the same way as the old code with Captum, and thus we don't need to change anything for it.
In https://github.com/pytorch/captum/pull/244, a bunch of of base functions were changed to class attributes in the captum/attr/_utils/attribution.py
file. If subclasses don't overwrite these attributes, then Sphinx v5.x displays them in the subclass' docs.
~~I think that they should probably be changed back to functions.~~
Edit:
I solve the issue by using :exclude-members:
in the rst docs.
I re-added the autodoc_process_docstring
function for Callable
& Any
, only this time without the sphinx ref code. Instead it replaces Callable
with ~typing.Callable
so that intersphinx hyperlinks it while preserving the italics formatting. The tilde '~
' character ensures that the typing.
portion is never displayed. No replacement will occur if Callable
or Any
are inside square brackets.
In Sphinx v4.3.2, this change will not be hyperlinked but the formatting & style will be correct. In later versions of Sphinx the hyperlinking works, while the style is preserved.
In addition to capitalizing Callable
& Any
, I think we should be capitalizing Iterator
, Iterable
, & Generator
well.
I also recently did one final sweep of the docstrings looking for mistakes and was able to fix the final few issues!
PyTorch now officially has the same docstring type guidelines as the ones that I've created in this PR for Captum, now that my PR has been merged!
https://github.com/pytorch/pytorch/pull/83536
https://github.com/pytorch/pytorch/blob/master/CONTRIBUTING.md#docstring-type-formatting
@NarineK @aobo-y This PR is ready for merging! Changes were made based on the reviewer feedback, and it would probably be best to merge this PR before other PRs end up conflicting with it
@ProGamerGov This PR has contained too many contents for me to directly review. Could you help me break down things a little bit.
Based on my understanding, the changes can be categorized into the following areas:
- typo, wording
- fix sphinx building errors/warnings
- docstring types
- as you have already noticed that we have a standardized styles now, do you think we need any updates here? To be clear, we are definitely not suggesting you to correct all existing errors. Just check if this PR will not diverge further from our style.
- add a function to ensure hyperlinks for python build-in types in web
Did I miss anything?
@aobo-y So in addition to what you listed, there are the following changes:
- Add type hints to the
len
functions:__len__(self) -> int:
, and to the__getitem__
functions. - Convert all
http
links tohttps
in docstrings for enhanced security & privacy. - Changed arxiv.org links to not link directly to the PDF.
- Added
ignore-members
to some rst files so that Sphinx 5.x and up is supported. - Changed the name of
docs/algorithms.md
todocs/attribution_algorithms.md
as Narine asked me to do in the email thread.
I can easily change all references of lowercase tensor
& tensors
in this PR if you wish. There are also a few instances where I changed list(int)
to list of int
, so I'll change them back to list[int]
.
Great! If the tensor
type thing is not too complicated, let's include it in this pr.
I just made the following changes, and now am working on changing the lowercase tensor to uppercase.
-
Converted all usages of the docstring types using
list of type
tolist[type]
:(list of Concept): (list of str): (list of str, optional): (list of int): (str, list of str): (str or list of str): (list of str or None, optional): (list of BaseFeature): (str, list of str, or Iterator): (list of Stat): (list of Dataset): (dict[Concept, list of str]):
-
Added missing comma to all docstring type references of:
(int, tuple, tensor or list, optional): # -> (int, tuple, tensor, or list, optional): (scalar, tensor, tuple of scalars or tensors, optional): # -> (scalar, tensor, tuple of scalars, or tensors, optional):
-
Added missing "
or
" to docstring types for all references of:(tensor, tuple of tensors, Callable): # -> `(tensor, tuple of tensors, or Callable):
-
Removed plural usage of "
scalars
".
@aobo-y Okay, I just converted all docstring type instances of tensor
& tensors
to Tensor
!
@aobo-y I just noticed a new warning in the libmamba test. I thought it might be causing the Conda test to fail, but it still seems to be working though there's an extra 10 minutes of install time now. I reported the issue to the Conda devs: https://github.com/conda/conda/issues/11790
All tests have passed!
I added the Tensor
intersphinx helper line to autodoc_process_docstring
as we have previously discussed. I also checked each replacement instance to ensure that there were no issues.
@aobo-y has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.