captum icon indicating copy to clipboard operation
captum copied to clipboard

Fix multiple Sphinx warnings & docstrings

Open ProGamerGov opened this issue 2 years ago • 19 comments

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

ProGamerGov avatar Jun 30 '22 01:06 ProGamerGov

I've come across some issues:

  • Doc variables with the lowercase any may be seen as referencing the any() function rather than the type. So, instead of any, we should probably be using the uppercase Any.

  • The callable type also has the same issue as any, as there is a callable() function.

  • Which example is the correct format for a tuple with a known size tuple of int, tuple(int, int), or tuple[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 of tuple 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)?

ProGamerGov avatar Jul 19 '22 20:07 ProGamerGov

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>`

ProGamerGov avatar Jul 20 '22 21:07 ProGamerGov

  • 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 avatar Jul 21 '22 06:07 NarineK

@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.

ProGamerGov avatar Jul 21 '22 13:07 ProGamerGov

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?

ProGamerGov avatar Jul 21 '22 21:07 ProGamerGov

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 avatar Jul 23 '22 04:07 NarineK

@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:

atlas_example1 atlas_example2

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 :

forward_any

And an example of Callable in FGSM: fgsm_callable

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.

ProGamerGov avatar Jul 23 '22 14:07 ProGamerGov

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 avatar Jul 26 '22 01:07 NarineK

@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.

ProGamerGov avatar Jul 26 '22 01:07 ProGamerGov

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 to bool, 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 either Tensor or torch.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 to tensor
  • 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 to dict. Libs like torchrec and torchaudio 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 avatar Jul 28 '22 01:07 aobo-y

@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.

ProGamerGov avatar Jul 28 '22 02:07 ProGamerGov

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

ProGamerGov avatar Jul 28 '22 21:07 ProGamerGov

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 Screen Shot 2022-07-29 at 3 29 29 PM

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 avatar Jul 29 '22 22:07 aobo-y

@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.

ProGamerGov avatar Jul 30 '22 00:07 ProGamerGov

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.

ProGamerGov avatar Jul 30 '22 16:07 ProGamerGov

@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.

sphinx_bug_fixed

Therefore, I've removed the autodoc_process_docstring in favor of updating the Sphinx version to v5.2.0 when it is released.

ProGamerGov avatar Jul 31 '22 20:07 ProGamerGov

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.

ProGamerGov avatar Aug 04 '22 20:08 ProGamerGov

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.

ProGamerGov avatar Aug 06 '22 20:08 ProGamerGov

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.

ProGamerGov avatar Aug 07 '22 17:08 ProGamerGov

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!

ProGamerGov avatar Aug 15 '22 20:08 ProGamerGov

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

ProGamerGov avatar Aug 19 '22 19:08 ProGamerGov

@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 avatar Aug 23 '22 15:08 ProGamerGov

@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 avatar Sep 01 '22 20:09 aobo-y

@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 to https 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 to docs/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].

ProGamerGov avatar Sep 01 '22 20:09 ProGamerGov

Great! If the tensor type thing is not too complicated, let's include it in this pr.

aobo-y avatar Sep 01 '22 21:09 aobo-y

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 to list[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".

ProGamerGov avatar Sep 01 '22 21:09 ProGamerGov

@aobo-y Okay, I just converted all docstring type instances of tensor & tensors to Tensor!

ProGamerGov avatar Sep 01 '22 21:09 ProGamerGov

@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!

ProGamerGov avatar Sep 01 '22 23:09 ProGamerGov

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.

ProGamerGov avatar Sep 02 '22 17:09 ProGamerGov

@aobo-y has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Sep 08 '22 00:09 facebook-github-bot