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.

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

Make it possible to use Kedro after uninstalling Rich. #3838

Merged
merged 57 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
3f39cd3
attempt to configure fallback logger
lrcouto Apr 24, 2024
80f0ff2
Set up fallback for logging on the logging function itself
lrcouto Apr 24, 2024
8c07db1
Lint
lrcouto Apr 24, 2024
6c66689
Lint
lrcouto Apr 24, 2024
d883b5d
Merge branch 'main' into remove-rich-as-mandatory-dependence
lrcouto Apr 25, 2024
e35cdaa
Merge branch 'main' into remove-rich-as-mandatory-dependence
SajidAlamQB Apr 30, 2024
bfbb1f1
Update kedro/logging.py
lrcouto Apr 30, 2024
2038792
Changes to rich variable on logging
lrcouto Apr 30, 2024
5b85711
Lint
lrcouto May 2, 2024
b75327d
attempt to configure fallback logger
lrcouto Apr 24, 2024
de86e8d
Set up fallback for logging on the logging function itself
lrcouto Apr 24, 2024
658d0c1
Lint
lrcouto Apr 24, 2024
609a0c2
Lint
lrcouto Apr 24, 2024
99bd891
Update kedro/logging.py
lrcouto Apr 30, 2024
a15bdcd
Changes to rich variable on logging
lrcouto Apr 30, 2024
e80d514
Lint
lrcouto May 2, 2024
f802384
adapt default logging
lrcouto May 3, 2024
b5aed65
Merge
lrcouto May 3, 2024
5061229
Update kedro/logging.py
lrcouto May 3, 2024
7c731f3
Create separate logging config file for rich
lrcouto May 6, 2024
c63c068
Merge branch 'remove-rich-as-mandatory-dependence' of github.com:kedr…
lrcouto May 6, 2024
a29220c
Hello linter my old friend
lrcouto May 6, 2024
f43c72c
Alternative for rich in kedro ipython
lrcouto May 6, 2024
8389023
Remove unnecessary try/except block
lrcouto May 6, 2024
1ea51e8
Update pyproject.toml
lrcouto May 7, 2024
6b98284
Merge branch 'main' into remove-rich-as-mandatory-dependence
lrcouto May 8, 2024
3663bdf
Resolve merge conflict
lrcouto May 8, 2024
115d339
Merge branch 'remove-rich-as-mandatory-dependence' of github.com:kedr…
lrcouto May 8, 2024
16e05b8
Lint
lrcouto May 8, 2024
269ec98
Fix config file paths
lrcouto May 8, 2024
5f3640b
Merge branch 'main' into remove-rich-as-mandatory-dependence
lrcouto May 9, 2024
235a5bf
Update kedro/ipython/__init__.py
lrcouto May 9, 2024
c1f6cbf
Update kedro/ipython/__init__.py
lrcouto May 9, 2024
210b0ed
Prevent kedro ipython from reimporting rich multiple times
lrcouto May 9, 2024
98f8e4d
Logging config
lrcouto May 9, 2024
191bba7
Change test config
lrcouto May 10, 2024
b44fb5e
Remove ' yEs ' tests
lrcouto May 10, 2024
c4c8aa7
Tests to cover ipython changes
lrcouto May 10, 2024
d76124f
Add test for import
lrcouto May 10, 2024
9dbaf94
Ignore test coverage on import try/except
lrcouto May 10, 2024
0c63233
Lint
lrcouto May 10, 2024
f3c65fa
Merge branch 'main' into remove-rich-as-mandatory-dependence
lrcouto May 10, 2024
1be4b05
Merge branch 'main' into remove-rich-as-mandatory-dependence
lrcouto May 14, 2024
4775fe4
Merge branch 'main' into remove-rich-as-mandatory-dependence
lrcouto May 14, 2024
671917d
Unpin cookiecutter version
lrcouto May 14, 2024
a8fea7a
Merge branch 'remove-rich-as-mandatory-dependence' of github.com:kedr…
lrcouto May 14, 2024
eb3b151
Add changes to documentation
lrcouto May 15, 2024
0f0214f
Update RELEASE.md
lrcouto May 15, 2024
192d349
Change RICH variable name to RICH_INTALLED
lrcouto May 15, 2024
28534ad
Merge branch 'remove-rich-as-mandatory-dependence' of github.com:kedr…
lrcouto May 15, 2024
02d8919
Merge branch 'main' into remove-rich-as-mandatory-dependence
lrcouto May 16, 2024
2074054
Merge branch 'main' into remove-rich-as-mandatory-dependence
lrcouto May 16, 2024
8c15acb
Add info about uninstalling rich on logging doc page
lrcouto May 16, 2024
6cf7d4d
Merge branch 'main' into remove-rich-as-mandatory-dependence
lrcouto May 20, 2024
61f44ce
Merge branch 'main' into remove-rich-as-mandatory-dependence
lrcouto May 20, 2024
6a41b4d
Merge branch 'main' into remove-rich-as-mandatory-dependence
lrcouto May 21, 2024
4f57399
Merge branch 'main' into remove-rich-as-mandatory-dependence
lrcouto May 21, 2024
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
1 change: 1 addition & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
include README.md
include LICENSE.md
include kedro/framework/project/default_logging.yml
include kedro/framework/project/rich_logging.yml
include kedro/ipython/*.png
include kedro/ipython/*.svg
recursive-include kedro/templates *
1 change: 1 addition & 0 deletions docs/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@
# "anchor not found" but it's a valid selector for code examples
"https://docs.delta.io/latest/delta-update.html#language-python",
"https://github.com/kedro-org/kedro/blob/main/kedro/framework/project/default_logging.yml",
"https://github.com/kedro-org/kedro/blob/main/kedro/framework/project/rich_logging.yml",
"https://github.com/kedro-org/kedro/blob/main/README.md#the-humans-behind-kedro", # "anchor not found" but is valid
"https://opensource.org/license/apache2-0-php/",
"https://docs.github.com/en/rest/overview/other-authentication-methods#via-username-and-password",
Expand Down
9 changes: 8 additions & 1 deletion kedro/framework/project/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,14 @@ def __init__(self) -> None:
# Check if the default logging configuration exists
default_logging_path = Path("conf/logging.yml")
if not default_logging_path.exists():
default_logging_path = Path(__file__).parent / "default_logging.yml"
default_logging_path = Path(
os.environ.get(
"KEDRO_LOGGING_CONFIG",
Path(__file__).parent / "rich_logging.yml"
if importlib.util.find_spec("rich")
else Path(__file__).parent / "default_logging.yml",
)
)

# Use the user path if available, otherwise, use the default path
if user_logging_path and Path(user_logging_path).exists():
Expand Down
27 changes: 20 additions & 7 deletions kedro/framework/project/default_logging.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,30 @@ version: 1

disable_existing_loggers: False

formatters:
simple:
format: "%(asctime)s - %(name)s - %(levelname)s - %(message)s"

handlers:
rich:
class: kedro.logging.RichHandler
rich_tracebacks: True
# Advance options for customisation.
# See https://docs.kedro.org/en/stable/logging/logging.html#project-side-logging-configuration
# tracebacks_show_locals: False
console:
class: logging.StreamHandler
level: INFO
formatter: simple
stream: ext://sys.stdout

info_file_handler:
class: logging.handlers.RotatingFileHandler
level: INFO
formatter: simple
filename: info.log
maxBytes: 10485760 # 10MB
backupCount: 20
encoding: utf8
delay: True

loggers:
kedro:
level: INFO

root:
handlers: [rich]
handlers: [console]
18 changes: 18 additions & 0 deletions kedro/framework/project/rich_logging.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
version: 1

disable_existing_loggers: False

handlers:
rich:
class: kedro.logging.RichHandler
rich_tracebacks: True
# Advance options for customisation.
# See https://docs.kedro.org/en/stable/logging/logging.html#project-side-logging-configuration
# tracebacks_show_locals: False

loggers:
kedro:
level: INFO

root:
handlers: [rich]
21 changes: 17 additions & 4 deletions kedro/ipython/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from __future__ import annotations

import importlib
import inspect
import logging
import os
Expand All @@ -18,8 +19,12 @@
from IPython.core.getipython import get_ipython
from IPython.core.magic import needs_local_scope, register_line_magic
from IPython.core.magic_arguments import argument, magic_arguments, parse_argstring
from rich.console import Console
from rich.syntax import Syntax

try:
import rich.console as rich_console
import rich.syntax as rich_syntax
except ImportError: # pragma: no cover
pass

from kedro.framework.cli import load_entry_points
from kedro.framework.cli.project import CONF_SOURCE_HELP, PARAMS_ARG_HELP
Expand All @@ -39,6 +44,8 @@

FunctionParameters = MappingProxyType

RICH = True if importlib.util.find_spec("rich") is not None else False
Copy link
Member

Choose a reason for hiding this comment

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

Maybe name this RICH_INSTALLED?



def load_ipython_extension(ipython: Any) -> None:
"""
Expand Down Expand Up @@ -281,8 +288,14 @@ def _create_cell_with_text(text: str, is_jupyter: bool = True) -> None:

def _print_cells(cells: list[str]) -> None:
for cell in cells:
Console().print("")
Console().print(Syntax(cell, "python", theme="monokai", line_numbers=False))
if RICH is True:
rich_console.Console().print("")
rich_console.Console().print(
rich_syntax.Syntax(cell, "python", theme="monokai", line_numbers=False)
)
else:
print("") # noqa: T201
print(cell) # noqa: T201


def _load_node(node_name: str, pipelines: _ProjectPipelines) -> list[str]:
Expand Down
4 changes: 0 additions & 4 deletions kedro/logging.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
"""
This module contains a logging handler class which produces coloured logs and tracebacks.
"""

import logging
import sys
from pathlib import Path
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ dependencies = [
"build>=0.7.0",
"cachetools>=4.1",
"click>=4.0",
"cookiecutter>=2.1.1,<3.0",
"cookiecutter>=2.1.1,<2.3", # 2.3 onwards depends on rich, which we want to avoid
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't pin cookiecutter so strictly, but instead clarify in docs that if users don't want to use rich they might have to downgrade their cookiecutter version.

"dynaconf>=3.1.2,<4.0",
"fsspec>=2021.4",
"gitpython>=3.0",
Expand Down
4 changes: 2 additions & 2 deletions tests/framework/cli/test_starters.py
Original file line number Diff line number Diff line change
Expand Up @@ -1119,7 +1119,7 @@ def test_invalid_tools_range(self, fake_kedro_cli, input):
message = f"'{input}' is an invalid range for project tools.\nPlease ensure range values go from smaller to larger."
assert message in result.output

@pytest.mark.parametrize("example_pipeline", ["y", "n", "N", "YEs", " yeS "])
@pytest.mark.parametrize("example_pipeline", ["y", "n", "N", "YEs"])
def test_valid_example(self, fake_kedro_cli, example_pipeline):
result = CliRunner().invoke(
fake_kedro_cli,
Expand Down Expand Up @@ -1285,7 +1285,7 @@ def test_invalid_tools_flag_combination(self, fake_kedro_cli, input):
in result.output
)

@pytest.mark.parametrize("example_pipeline", ["y", "n", "N", "YEs", " yeS "])
@pytest.mark.parametrize("example_pipeline", ["y", "n", "N", "YEs"])
def test_valid_example(self, fake_kedro_cli, example_pipeline):
"""Test project created from config."""
config = {
Expand Down
15 changes: 13 additions & 2 deletions tests/ipython/test_ipython.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,9 +457,20 @@ def test_load_node_with_ipython(self, mocker, ipython, run_env):
ipython.magic("load_node dummy_node")
spy.assert_called_once()

@pytest.mark.parametrize("run_env", ["databricks", "colab", "dummy"])
def test_load_node_with_other(self, mocker, ipython, run_env):
@pytest.mark.parametrize(
"run_env, rich_installed",
[
("databricks", True),
("databricks", False),
("colab", True),
("colab", False),
("dummy", True),
("dummy", False),
],
)
def test_load_node_with_other(self, mocker, ipython, run_env, rich_installed):
mocker.patch("kedro.ipython._find_kedro_project")
mocker.patch("kedro.ipython.RICH", rich_installed)
mocker.patch("kedro.ipython._load_node", return_value=["cell1", "cell2"])
mocker.patch("kedro.ipython._guess_run_environment", return_value=run_env)
spy = mocker.spy(kedro.ipython, "_print_cells")
Expand Down