Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MAINTENANCE] Typing render/view #8903

Merged
merged 12 commits into from
Nov 1, 2023
Merged

[MAINTENANCE] Typing render/view #8903

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3e7a11a
view
Kilo59 Oct 31, 2023
0774d12
view typing
Kilo59 Oct 31, 2023
b0489a0
move legacy code to exclude section
Kilo59 Oct 31, 2023
f325238
ignore 2 notebook_renderer warnings
Kilo59 Oct 31, 2023
4b04981
import `Template` as `jTemplate`
Kilo59 Oct 31, 2023
eddad7d
remove `NoOpTemplate`
Kilo59 Oct 31, 2023
6d7f17b
Merge branch 'develop' into m/_/10_31_mob_typing
Kilo59 Oct 31, 2023
e2733aa
Merge branch 'develop' into m/_/10_31_mob_typing
Kilo59 Nov 1, 2023
420a4d6
missed one
Kilo59 Nov 1, 2023
10f0fde
missed typing
Kilo59 Nov 1, 2023
2836c7e
missed one
Kilo59 Nov 1, 2023
c9d098b
whitespace
Kilo59 Nov 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions great_expectations/render/renderer/notebook_renderer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import nbformat

from great_expectations.compatibility.typing_extensions import override
from great_expectations.render.renderer.renderer import Renderer
from great_expectations.util import (
convert_json_string_to_be_python_compliant,
Expand Down Expand Up @@ -48,7 +49,7 @@ def add_code_cell(
code = lint_code(code).rstrip("\n")

cell = nbformat.v4.new_code_cell(code)
self._notebook["cells"].append(cell)
self._notebook["cells"].append(cell) # type: ignore[index] # _notebook could be None

def add_markdown_cell(self, markdown: str) -> None:
"""
Expand All @@ -60,7 +61,7 @@ def add_markdown_cell(self, markdown: str) -> None:
Nothing, adds a cell to the class instance notebook
"""
cell = nbformat.v4.new_markdown_cell(markdown)
self._notebook["cells"].append(cell)
self._notebook["cells"].append(cell) # type: ignore[index] # _notebook could be None

@classmethod
def write_notebook_to_disk(
Expand All @@ -75,6 +76,7 @@ def write_notebook_to_disk(
with open(notebook_file_path, "w") as f:
nbformat.write(notebook, f)

@override
def render(self, **kwargs: dict) -> nbformat.NotebookNode:
"""
Render a notebook from parameters.
Expand Down
39 changes: 23 additions & 16 deletions great_expectations/render/view/view.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
from __future__ import annotations

import datetime
import json
import re
from collections import OrderedDict
from string import Template as pTemplate
from typing import TYPE_CHECKING, ClassVar
from uuid import uuid4

import mistune
Expand All @@ -14,8 +17,10 @@
select_autoescape,
)

from great_expectations.compatibility.typing_extensions import override

try:
from jinja2 import contextfilter
from jinja2 import contextfilter # type: ignore[attr-defined] # for jinja 2.0
except ImportError:
from jinja2 import pass_context as contextfilter

Expand All @@ -26,10 +31,9 @@
RenderedDocumentContent,
)


class NoOpTemplate:
def render(self, document):
return document
if TYPE_CHECKING:
from jinja2 import BaseLoader
from jinja2 import Template as jTemplate


class PrettyPrintTemplate:
Expand All @@ -55,7 +59,7 @@ class DefaultJinjaView:

"""

_template = NoOpTemplate
_template: ClassVar[str]

def __init__(
self, custom_styles_directory=None, custom_views_directory=None
Expand All @@ -66,7 +70,7 @@ def __init__(
templates_loader = PackageLoader("great_expectations", "render/view/templates")
styles_loader = PackageLoader("great_expectations", "render/view/static/styles")

loaders = [templates_loader, styles_loader]
loaders: list[BaseLoader] = [templates_loader, styles_loader]
if self.custom_styles_directory:
loaders.append(FileSystemLoader(self.custom_styles_directory))
if self.custom_views_directory:
Expand Down Expand Up @@ -109,11 +113,8 @@ def render(self, document, template=None, **kwargs):
document = document.to_json_dict()
return t.render(document, **kwargs)

def _get_template(self, template):
if template is None:
return NoOpTemplate

template = self.env.get_template(template)
def _get_template(self, template_str: str) -> jTemplate:
template = self.env.get_template(template_str)
template.globals["now"] = lambda: datetime.datetime.now(datetime.timezone.utc)

return template
Expand Down Expand Up @@ -187,7 +188,7 @@ def render_content_block( # noqa: PLR0911, PLR0913, PLR0912
template_filename = f"markdown_{content_block_type}.j2"
else:
template_filename = f"{content_block_type}.j2"
template = self._get_template(template=template_filename)
template = self._get_template(template_str=template_filename)
if content_block_id:
return template.render(
jinja_context,
Expand Down Expand Up @@ -435,6 +436,7 @@ def _validate_document(self, document) -> None:
class DefaultJinjaPageView(DefaultJinjaView):
_template = "page.j2"

@override
def _validate_document(self, document) -> None:
assert isinstance(document, RenderedDocumentContent)

Expand All @@ -446,6 +448,7 @@ class DefaultJinjaIndexPageView(DefaultJinjaPageView):
class DefaultJinjaSectionView(DefaultJinjaView):
_template = "section.j2"

@override
def _validate_document(self, document) -> None:
assert isinstance(
document["section"], dict
Expand All @@ -455,6 +458,7 @@ def _validate_document(self, document) -> None:
class DefaultJinjaComponentView(DefaultJinjaView):
_template = "component.j2"

@override
def _validate_document(self, document) -> None:
assert isinstance(
document["content_block"], dict
Expand All @@ -466,7 +470,8 @@ class DefaultMarkdownPageView(DefaultJinjaView):
Convert a document to markdown format.
"""

def _validate_document(self, document: RenderedDocumentContent) -> bool:
@override
def _validate_document(self, document: RenderedDocumentContent) -> None:
"""
Validate that the document is of the appropriate type at runtime.
"""
Expand All @@ -490,7 +495,8 @@ def render(self, document, template=None, **kwargs):
else:
return super().render(document=document, template=template, **kwargs)

def render_string_template(self, template: pTemplate) -> pTemplate:
@override
def render_string_template(self, template: pTemplate) -> pTemplate | str:
"""
Render string for markdown rendering. Bold all parameters and perform substitution.
Args:
Expand Down Expand Up @@ -547,10 +553,11 @@ def render_string_template(self, template: pTemplate) -> pTemplate:
"$PARAMETER", "$$PARAMETER"
)

return pTemplate(template.get("template")).safe_substitute(
return pTemplate(template["template"]).safe_substitute(
template.get("params", {})
)

@override
@contextfilter
def render_content_block( # noqa: PLR0913
self,
Expand Down
27 changes: 13 additions & 14 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,24 @@ exclude = [
'cli/upgrade_helpers/upgrade_helper_v13\.py', # 17 - This is legacy code and will not be typed.
'dataset/sparkdf_dataset\.py', # 3 - This is legacy code and will not be typed.
'dataset/sqlalchemy_dataset\.py', # 16 - This is legacy code and will not be typed.
'core/usage_statistics/anonymizers/batch_anonymizer\.py', # 10 - This code will be removed in 1.0
'core/usage_statistics/anonymizers/batch_request_anonymizer\.py', # 16 - This code will be removed in 1.0
'core/usage_statistics/anonymizers/checkpoint_anonymizer\.py', # 16 - This code will be removed in 1.0
'core/usage_statistics/anonymizers/data_docs_anonymizer\.py', # 5 - This code will be removed in 1.0
'core/usage_statistics/anonymizers/datasource_anonymizer\.py', # 9 - This code will be removed in 1.0
'core/usage_statistics/anonymizers/expectation_anonymizer\.py', # 6 - This code will be removed in 1.0
'core/usage_statistics/anonymizers/validation_operator_anonymizer\.py', # 5 - This code will be removed in 1.0
'render/renderer/v3/suite_edit_notebook_renderer\.py', # 11 - This is legacy code and will not be typed.
'render/renderer/v3/suite_profile_notebook_renderer\.py', # 4 - This is legacy code and will not be typed.
'render/renderer/suite_edit_notebook_renderer\.py', # 7 - This is legacy code and will not be typed.
'render/renderer/suite_scaffold_notebook_renderer\.py', # 7 - This is legacy code and will not be typed.
'render/renderer/datasource_new_notebook_renderer\.py', # 4 - This is legacy code and will not be typed.
Comment on lines +83 to +87
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NathanFarmer do these look right? Am I missing anything?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great_expectations/render/renderer/notebook_renderer.py is now typed but should it be deleted when we remove the cli?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just missing: render/renderer/checkpoint_new_notebook_renderer.py

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Will update both.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think notebook_renderer.py will also be removed with the CLI removal. It is the base class for the other notebook renderers.

# END ALWAYS EXCLUDE SECTION ######################################################
#
# #################################################################################
# TODO: complete typing for the following modules and remove from exclude list
# number is the current number of typing errors for the excluded pattern
'core/usage_statistics/anonymizers/batch_anonymizer\.py', # 10
'core/usage_statistics/anonymizers/batch_request_anonymizer\.py', # 16
'core/usage_statistics/anonymizers/checkpoint_anonymizer\.py', # 16
'core/usage_statistics/anonymizers/data_docs_anonymizer\.py', # 5
'core/usage_statistics/anonymizers/datasource_anonymizer\.py', # 9
'core/usage_statistics/anonymizers/expectation_anonymizer\.py', # 6
'core/usage_statistics/anonymizers/validation_operator_anonymizer\.py', # 5

'expectations/core/expect_column_values_to_be_of_type\.py', # 12
'expectations/core/expect_column_values_to_not_match_regex_list\.py', # 2
'expectations/core/expect_column_values_to_not_match_regex\.py', # 2
Expand Down Expand Up @@ -136,17 +142,10 @@ exclude = [
'render/renderer/checkpoint_new_notebook_renderer\.py', # 9
Kilo59 marked this conversation as resolved.
Show resolved Hide resolved
'render/renderer/content_block/content_block\.py', # 5
'render/renderer/content_block/exception_list_content_block\.py', # 4
'render/renderer/datasource_new_notebook_renderer\.py', # 4
'render/renderer/notebook_renderer\.py', # 2
'render/renderer/page_renderer\.py', # 10
'render/renderer/profiling_results_overview_section_renderer\.py', # 2
'render/renderer/site_builder\.py', # 3
'render/renderer/slack_renderer\.py', # 9
'render/renderer/suite_edit_notebook_renderer\.py', # 7
'render/renderer/suite_scaffold_notebook_renderer\.py', # 7
'render/renderer/v3/suite_edit_notebook_renderer\.py', # 11
'render/renderer/v3/suite_profile_notebook_renderer\.py', # 4
'render/view/view\.py', # 11
'rule_based_profiler/domain_builder/map_metric_column_domain_builder\.py', # 8
'rule_based_profiler/estimators/bootstrap_numeric_range_estimator\.py', # 8
'rule_based_profiler/estimators/kde_numeric_range_estimator\.py', # 7
Expand Down
Loading