API management and documentation mechanisms #1701

vshampor · 2023-04-12T13:08:11Z

Changes

Added a decorator for marking entities as API and a set of Github actions to check whether a PR changes API (if so, then the PR will be automatically labeled as such) and to generate and push API documentation automatically to Github pages using Sphinx, AutoAPI (by readthedocs) and custom API name collection code.

Reason for changes

Both devs and users will finally have an understanding of what is considered API and what is not, and have an opportunity to browse API documentation without having to look into source code.

Related tickets

99696

Tests

N/A

vshampor · 2023-04-13T16:31:53Z

I had to experiment on my fork to properly debug the Github actions. The end result on Github pages would look like this:
https://vshampor.github.io/nncf/

(It used to be that this page contained only the schema, and now it has been extended to API docs. The schema is still there at https://vshampor.github.io/nncf/schema)

The page contains only the description of entities that were decorated with @api() and of the containing Python modules in the import path. The module descriptions are currently licenses, because we actually currently use the module docstrings to store licenses (will fix this by moving license content to comments and setting up proper docstrings for API-impacted modules). The class and function descriptions may have invalid formatting in HTML and missing descriptions - all this is fixable by adding or reformatting corresponding docstrings which will be done in a follow-up PR.

For demo of auto PR labeling, see:
vshampor#6 - modification of an API function signature, PR was marked as API automatially
vshampor#7 - addition of an API entity when compared to the develop state, PR was marked as API automatically
vshampor#8 - a change inside an API function that does not explicitly impact API as seen by the external user, PR was not marked as API automatically - it is up to reviewers to determine whether such PR changes API behaviour or not.

NB: automatic PR labeling basically works by comparing API reference HTML content between develop and the state in a given PR.

andrey-churkin · 2023-04-17T09:59:15Z

I think we should also include

nncf.parameters.TargetDevice
nncf.parameters.ModelType
nncf.scopes.IgnoredScope
nncf.data.dataset.Dataset

vshampor · 2023-04-17T12:13:15Z

I think we should also include

nncf.parameters.TargetDevice

nncf.parameters.ModelType

nncf.scopes.IgnoredScope

nncf.data.dataset.Dataset

Done

vshampor · 2023-04-17T12:50:14Z

docs/api/source/_autoapi_templates/python/attribute.rst

@@ -0,0 +1 @@
+{% extends "python/data.rst" %}


These and other .rst files control the layout of the documentation and are copy-and-pasted from defaults, for future adjustments if necessary.

vshampor · 2023-04-17T12:50:46Z

docs/api/source/conf.py

@@ -0,0 +1,114 @@
+import importlib


This file controls the documentation build process that is executed during sphinx-build.

vshampor · 2023-04-17T12:51:20Z

docs/api/source/conf.py

+
+
+with mock(['torch', 'torchvision', 'onnx', 'onnxruntime', 'openvino', 'tensorflow', 'tensorflow_addons']):
+    api_fqns = collect_api_entities()


This code allows to collect API entities without having to install all backends at once.

AlexKoff88 · 2023-04-19T10:53:41Z

It looks great really, thanks @vshampor . I have three major comments:

Can we remove the license from every page? It reduces the useful screen space and you need to scroll every time.
It would be great to have examples of configs along with descriptions of the features. For example, if we look at the KD description here, having a snippet of the config that enables this increases the usefulness of this page a lot, IMO.
The pages do not reflect the fact that some of the APIs are exposed at the top level of the "nncf" namespace. For example, if to look at the quantize API here, it is mentioned that it is a part of the "nncf.quantization.quantize" namespace but it can be imported directly from the "nncf". One way to work around it is to provide more documentation.

I think some follow up issues and PRs should be merged to address the last two issues.

vshampor · 2023-04-19T12:08:59Z

@AlexKoff88

Can we remove the license from every page? It reduces the useful screen space and you need to scroll every time.

Sure, will do this in a follow-up PR along with docstring formatting adjustments.

It would be great to have examples of configs along with descriptions of the features. For example, if we look at the KD description here, having a snippet of the config that enables this increases the usefulness of this page a lot, IMO.

Basically everything that can be written in a docstring will be rendered, so there should be no problem to add config snippets and even code snippets via doctest later.

The pages do not reflect the fact that some of the APIs are exposed at the top level of the "nncf" namespace. For example, if to look at the quantize API here, it is mentioned that it is a part of the "nncf.quantization.quantize" namespace but it can be imported directly from the "nncf". One way to work around it is to provide more documentation.

I'll see if I can add another wrapper function to do this via code, maybe in this PR even.

vshampor · 2023-04-19T13:42:12Z

nncf/quantization/quantize.py

@@ -26,6 +27,7 @@
 from nncf.parameters import TargetDevice


+@api(canonical_alias="nncf.quantize")


@AlexKoff88 check this out - in this way it can be specified that the canonical import name should be "nncf.quantize", and the docs will be generated with the canonical import name in mind. Note that this also requires that the symbol itself is imported in corresponding __init__.py file (in this case, nncf/__init__.py), otherwise the doc build will fail and the doc build check will be red. I regenerated the page at https://vshampor.github.io/nncf/index.html so that you can see the effect.

I cannot find quantize() in the latest version of the API build, only quantize_with_accuracy_control().

Fixed - make sure to reload the page with Ctrl-Shift-R so that you don't get a cached version.

One of the reasons was that had a function quantize exposed in __init__.py in the same subpackage that also contained the module called quantize.py. This meant that the quantize.py file was effectively impossible to import from, apart from the exposed quantize and quantize_with_accuracy_control functions, and this broke down the doc search mechanism, and is anyway considered to be bad practice (since the imported name shadows the subpackage name). Had to rename the quantize.py to quantize_fns.py, if you have suggestions for a better name, please post them.

@vshampor I would recommend to rename quantize.py -> quantize_model.py because torch has already quantize_functions.py https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/quantization/quantize_functions.py

alexsu52

please, rebase PR.

…onical names

vshampor force-pushed the api_doc_gen branch 6 times, most recently from b668f1c to e686418 Compare April 13, 2023 15:16

github-actions bot removed the NNCF OpenVINO Pull requests that updates NNCF OpenVINO label Apr 13, 2023

vshampor changed the title ~~Api doc gen~~ API management and documentation mechanisms Apr 13, 2023

vshampor marked this pull request as ready for review April 13, 2023 16:11

vshampor requested review from AlexKoff88, alexsu52, nikita-malininn and andrey-churkin April 13, 2023 16:32

vshampor force-pushed the api_doc_gen branch from d66ba03 to bf55ebc Compare April 14, 2023 13:07

vshampor force-pushed the api_doc_gen branch from 85d5191 to bf581c6 Compare April 17, 2023 12:49

vshampor commented Apr 17, 2023

View reviewed changes

vshampor force-pushed the api_doc_gen branch from bf581c6 to 73ee28c Compare April 19, 2023 09:28

vshampor force-pushed the api_doc_gen branch from 73ee28c to 7c6f63b Compare April 19, 2023 13:39

vshampor commented Apr 19, 2023

View reviewed changes

alexsu52 approved these changes Apr 26, 2023

View reviewed changes

vshampor force-pushed the api_doc_gen branch 2 times, most recently from 9cb5ecf to 6956f04 Compare April 26, 2023 12:06

vshampor added 22 commits April 26, 2023 16:20

Sphinx build with recursive autosummary and custom template

b68ef96

Decorator works to skip non-decorated classes

6b9fe7f

Recursive module search

f42bf06

Custom API entity collector

c7a9f18

Proper rendering

3292327

Mark some entities as API

982f664

AutoAPI + FQN collection

efecafa

Switch to book theme and improve formatting

ccf0a90

Mock TF imports

0ca99cd

Comments

277ca59

Add GH workflows for doc builds and API checks

de01ee1

Pylint

a7302bf

Fix diff files check

1242e8e

Fix torch version parsing

fb0a154

Fix pull request targeting

6070aa4

Fix derived class handling

8c56c36

Add more API entities

d16c853

Support import aliasing

5521f68

Detect duplicate canonical aliases

5102949

Fix module hiding for quantize

fb328ed

Set skip to False for API entities

6866c9f

Move type hints to doc body and override NNCF API type hints with can…

299ff61

…onical names

vshampor force-pushed the api_doc_gen branch from 6956f04 to 1b05aca Compare April 26, 2023 14:22

Fix rebase

a777a50

vshampor force-pushed the api_doc_gen branch from 1b05aca to a777a50 Compare April 26, 2023 15:00

alexsu52 merged commit 211a8a5 into openvinotoolkit:develop Apr 27, 2023

vshampor deleted the api_doc_gen branch January 9, 2024 13:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API management and documentation mechanisms #1701

API management and documentation mechanisms #1701

vshampor commented Apr 12, 2023 •

edited

Loading

vshampor commented Apr 13, 2023

andrey-churkin commented Apr 17, 2023 •

edited

Loading

vshampor commented Apr 17, 2023

vshampor Apr 17, 2023

vshampor Apr 17, 2023

vshampor Apr 17, 2023

AlexKoff88 commented Apr 19, 2023

vshampor commented Apr 19, 2023

vshampor Apr 19, 2023 •

edited

Loading

AlexKoff88 Apr 21, 2023

vshampor Apr 21, 2023

alexsu52 Apr 25, 2023 •

edited

Loading

alexsu52 left a comment



		with mock(['torch', 'torchvision', 'onnx', 'onnxruntime', 'openvino', 'tensorflow', 'tensorflow_addons']):
		api_fqns = collect_api_entities()

		@@ -26,6 +27,7 @@
		from nncf.parameters import TargetDevice


		@api(canonical_alias="nncf.quantize")

API management and documentation mechanisms #1701

API management and documentation mechanisms #1701

Conversation

vshampor commented Apr 12, 2023 • edited Loading

Changes

Reason for changes

Related tickets

Tests

vshampor commented Apr 13, 2023

andrey-churkin commented Apr 17, 2023 • edited Loading

vshampor commented Apr 17, 2023

vshampor Apr 17, 2023

Choose a reason for hiding this comment

vshampor Apr 17, 2023

Choose a reason for hiding this comment

vshampor Apr 17, 2023

Choose a reason for hiding this comment

AlexKoff88 commented Apr 19, 2023

vshampor commented Apr 19, 2023

vshampor Apr 19, 2023 • edited Loading

Choose a reason for hiding this comment

AlexKoff88 Apr 21, 2023

Choose a reason for hiding this comment

vshampor Apr 21, 2023

Choose a reason for hiding this comment

alexsu52 Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

alexsu52 left a comment

Choose a reason for hiding this comment

vshampor commented Apr 12, 2023 •

edited

Loading

andrey-churkin commented Apr 17, 2023 •

edited

Loading

vshampor Apr 19, 2023 •

edited

Loading

alexsu52 Apr 25, 2023 •

edited

Loading