Repository: great-expectations/great_expectations
Pull request #6831 (2023-01-18 21:21:31)

[MAINTENANCE] Use shortened dotted paths in api docs
Changes proposed in this pull request:
- Use shortened paths in API docs. E.g. if a class is imported in a parent `__init__.py` file use that instead of the file with the class definition. For example, `great_expectations.core.ExpectationSuite` instead of `great_expectations.core.expectation_suite.ExpectationSuite`. This appears in API docs in the class definitions.

### Definition of Done

- [x] My code follows the Great Expectations [style guide](https://docs.greatexpectations.io/docs/contributing/style_guides/code_style)
- [x] I have performed a [self-review](https://docs.greatexpectations.io/docs/contributing/contributing_checklist) of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added [unit tests](https://docs.greatexpectations.io/docs/contributing/contributing_test#writing-unit-and-integration-tests) where applicable and made sure that new and existing tests are passing.
- [x] I have run any local integration tests and made sure that nothing is broken.

netlify[bot]: ### <span aria-hidden="true">👷</span> Deploy Preview for *niobium-lead-7998* processing.


|  Name | Link |
|---------------------------------|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 7d6192af933ded68593dbfedbfac403b7e5bf5a7 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/sites/niobium-lead-7998/deploys/63c862dd6e9348000991ea2c |
codesee-maps[bot]: <details>
<summary><b>👇 <a href="https://app.codesee.io/r/reviews?pr=6831&src=https%3A%2F%2Fgithub.com%2Fgreat-expectations%2Fgreat_expectations">Click on the image</a> for a new way to code review</b>
</summary>

  - Make big changes easier — review code in small groups of related files
  - Know where to start — see the whole change at a glance
  - Take a code tour — explore the change with an interactive tour
  - Make comments and review — all fully sync’ed with github

    [Try it now!](https://app.codesee.io/r/reviews?pr=6831&src=https%3A%2F%2Fgithub.com%2Fgreat-expectations%2Fgreat_expectations)
</details>

[![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/great-expectations/great_expectations/6831/0e456e42/7d6192af933ded68593dbfedbfac403b7e5bf5a7.svg)](https://app.codesee.io/r/reviews?pr=6831&src=https%3A%2F%2Fgithub.com%2Fgreat-expectations%2Fgreat_expectations)

#### Legend
![CodeSee Map Legend](https://app.codesee.io/Legend_v2.png)

Patch:
diff --git a/docs/sphinx_api_docs_source/build_sphinx_api_docs.py b/docs/sphinx_api_docs_source/build_sphinx_api_docs.py
index a062a3235..6f77f314f 100644
--- a/docs/sphinx_api_docs_source/build_sphinx_api_docs.py
+++ b/docs/sphinx_api_docs_source/build_sphinx_api_docs.py
@@ -26,7 +26,6 @@ import logging
 import os
 import pathlib
 import shutil
-import urllib
 from typing import Tuple
 from urllib.parse import urlparse
 
@@ -38,7 +37,7 @@ from scripts.check_public_api_docstrings import (
     get_public_api_definitions,
     get_public_api_module_level_function_definitions,
 )
-from scripts.public_api_report import Definition
+from scripts.public_api_report import Definition, get_shortest_dotted_path
 
 logger = logging.getLogger(__name__)
 logger.addHandler(logging.StreamHandler())
@@ -242,7 +241,7 @@ class SphinxInvokeDocsBuilder:
         with filepath.open("a") as f:
             f.write(stub)
 
-    def _build_module_md_stubs(self):
+    def _build_module_md_stubs(self) -> None:
         """Build markdown stub files with rst directives for auto documenting modules."""
 
         definitions = get_public_api_module_level_function_definitions()
@@ -256,42 +255,20 @@ class SphinxInvokeDocsBuilder:
                 ),
             )
 
-    def _get_entity_name(self, definition: Definition):
+    def _get_entity_name(self, definition: Definition) -> str:
         """Get the name of the entity (class, module, function)."""
         return definition.ast_definition.name
 
-    def _get_md_file_name_from_entity_name(self, definition: Definition):
+    def _get_md_file_name_from_entity_name(self, definition: Definition) -> str:
         """Generate markdown file name from the entity definition."""
         snake_name = camel_to_snake(self._get_entity_name(definition=definition))
         return f"{snake_name}.md"
 
-    def _get_md_file_name_from_dotted_path_prefix(self, definition: Definition):
+    def _get_md_file_name_from_dotted_path_prefix(self, definition: Definition) -> str:
         """Generate markdown file name from the dotted path prefix."""
         dotted_path_prefix = self._get_dotted_path_prefix(definition=definition)
         return f"{dotted_path_prefix}.md"
 
-    def _get_dotted_path_to_class(self, definition: Definition):
-        """Get the dotted path to a class.
-
-        e.g. great_expectations.core.expectation_suite.ExpectationSuite
-        """
-        dotted_path_prefix = self._get_dotted_path_prefix(definition=definition)
-
-        class_name = self._get_entity_name(definition=definition)
-        dotted_path = f"{dotted_path_prefix}.{class_name}"
-
-        # Note: shortened_dotted_paths is temporary,
-        # to be replaced with automated method using AST parsing:
-        shortened_dotted_paths = {
-            "great_expectations.data_context.data_context.abstract_data_context.AbstractDataContext": "great_expectations.data_context.AbstractDataContext",
-            "great_expectations.core.expectation_suite.ExpectationSuite": "great_expectations.core.ExpectationSuite",
-        }
-
-        if dotted_path in shortened_dotted_paths:
-            return shortened_dotted_paths[dotted_path]
-
-        return dotted_path
-
     def _get_dotted_path_prefix(self, definition: Definition):
         """Get the dotted path up to the class or function name."""
         path = definition.filepath
@@ -302,7 +279,9 @@ class SphinxInvokeDocsBuilder:
     def _create_class_md_stub(self, definition: Definition) -> str:
         """Create the markdown stub content for a class."""
         class_name = self._get_entity_name(definition=definition)
-        dotted_import = self._get_dotted_path_to_class(definition=definition)
+        dotted_import = get_shortest_dotted_path(
+            definition=definition, repo_root_path=self.repo_root
+        )
         return f"""# {class_name}
 
 ```{{eval-rst}}
diff --git a/scripts/public_api_report.py b/scripts/public_api_report.py
index 011e80cea..49e17e983 100644
--- a/scripts/public_api_report.py
+++ b/scripts/public_api_report.py
@@ -374,6 +374,77 @@ class CodeParser:
         return function_definitions
 
 
+def get_shortest_dotted_path(
+    definition: Definition, repo_root_path: pathlib.Path
+) -> str:
+    """Get the shortest dotted path to a class definition.
+
+    e.g. if a class is imported in a parent __init__.py file use that
+    instead of the file with the class definition.
+
+    Args:
+        definition: Class definition.
+        repo_root_path: Repository root path to make sure paths are relative.
+
+    Returns:
+        Dotted representation of shortest import path
+            e.g. great_expectations.core.ExpectationSuite
+    """
+
+    if definition.filepath.is_absolute():
+        relative_definition_path = definition.filepath.relative_to(repo_root_path)
+    else:
+        relative_definition_path = definition.filepath
+
+    # Traverse parent folders from definition.filepath
+    shortest_path_prefix = str(".".join(relative_definition_path.parts)).replace(
+        ".py", ""
+    )
+    shortest_path = f"{shortest_path_prefix}.{definition.name}"
+
+    path_parts = list(relative_definition_path.parts)
+    while path_parts:
+        # Keep traversing, shortening path if shorter path is found.
+        path_parts.pop()
+        # if __init__.py is found, ast parse and check for import of the class
+        init_path = repo_root_path / pathlib.Path(*path_parts, "__init__.py")
+        if init_path.is_file():
+
+            import_names = []
+            with open(init_path) as f:
+                file_contents: str = f.read()
+
+            import_names.extend(_get_import_names(file_contents))
+
+            # If name is found in imports, shorten path to where __init__.py is found
+            if definition.name in import_names:
+                shortest_path_prefix = str(".".join(path_parts))
+                shortest_path = f"{shortest_path_prefix}.{definition.name}"
+
+    return shortest_path
+
+
+def _get_import_names(code: str) -> List[str]:
+    """Get import names from import statements.
+
+    Args:
+        code: Code with imports to parse.
+
+    Returns:
+        Flattened list of names from imports.
+    """
+
+    tree = ast.parse(code)
+
+    import_names = []
+    for node in ast.walk(tree):
+        if isinstance(node, (ast.Import, ast.ImportFrom)):
+            for alias in node.names:
+                import_names.append(alias.name)
+
+    return import_names
+
+
 class PublicAPIChecker:
     """Check if functions, methods and classes are marked part of the PublicAPI."""
 


Test patch:
diff --git a/tests/scripts/test_public_api_report.py b/tests/scripts/test_public_api_report.py
index 80e4dbcc5..7ba75ca83 100644
--- a/tests/scripts/test_public_api_report.py
+++ b/tests/scripts/test_public_api_report.py
@@ -13,6 +13,8 @@ from scripts.public_api_report import (
     IncludeExcludeDefinition,
     PublicAPIChecker,
     PublicAPIReport,
+    _get_import_names,
+    get_shortest_dotted_path,
 )
 
 
@@ -268,6 +270,67 @@ class TestCodeParser:
         }
 
 
+def test_get_shortest_dotted_path(monkeypatch):
+    """Test path traversal using an example file.
+
+    The example is just any class that is imported in a __init__.py file in a
+    parent folder. This test can be modified to use any such import e.g. if the
+    example file changes, or a fixture created so the example isn't coming
+    from the repo.
+    """
+    repo_root = pathlib.Path(__file__).parent.parent.parent
+    monkeypatch.chdir(repo_root)
+    filepath = pathlib.Path("great_expectations/core/expectation_suite.py")
+    definition = Definition(
+        name="ExpectationSuite", filepath=filepath, ast_definition=ast.ClassDef()
+    )
+
+    # This is the actual path
+    assert (
+        get_shortest_dotted_path(definition=definition, repo_root_path=repo_root)
+        != "great_expectations.core.expectation_suite.ExpectationSuite"
+    )
+    # This is the shortest path
+    assert (
+        get_shortest_dotted_path(definition=definition, repo_root_path=repo_root)
+        == "great_expectations.core.ExpectationSuite"
+    )
+
+
+@pytest.fixture
+def various_imports() -> str:
+    return """import some_module
+import SomeClass
+from some_module import ClassFromSomeModule
+from some_other_module import ClassFromSomeOtherModule
+import AliasClass as ac
+import alias_module as am
+from alias_module import AliasClassFromAliasModule as acfam
+from a.b.c import some_method as sm
+"""
+
+
+def test__get_import_names(various_imports: str):
+    """Make sure the actual class and module names are returned."""
+    tree = ast.parse(various_imports)
+    import_names = []
+
+    for node in ast.walk(tree):
+        if isinstance(node, (ast.Import, ast.ImportFrom)):
+            import_names.extend(_get_import_names(node))
+
+    assert import_names == [
+        "some_module",
+        "SomeClass",
+        "ClassFromSomeModule",
+        "ClassFromSomeOtherModule",
+        "AliasClass",
+        "alias_module",
+        "AliasClassFromAliasModule",
+        "some_method",
+    ]
+
+
 @pytest.fixture
 def public_api_checker(
     docs_example_parser: DocsExampleParser, code_parser: CodeParser