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

Plain logging contains rich color codes #3626

Closed
sbrugman opened this issue Feb 17, 2024 · 4 comments
Closed

Plain logging contains rich color codes #3626

sbrugman opened this issue Feb 17, 2024 · 4 comments
Labels
Component: Logging Issue: Feature Request New feature or improvement to existing feature

Comments

@sbrugman
Copy link
Contributor

Description

Following the instructions here, the logging will still outputs color codes.

Expected Result

Loading data from datasetname (MyDataset).

Actual Result

Loading data from [dark_orange]datasetname[/dark_orange] (MyDataset).

Your Environment

  • Kedro version used (0.19.2):
@noklam noklam added this to the Something about logging milestone Feb 26, 2024
@noklam noklam added the Issue: Feature Request New feature or improvement to existing feature label Feb 26, 2024
@noklam
Copy link
Contributor

noklam commented Feb 26, 2024

@AhdraMeraliQB Do you have any idea here?

@sbrugman
Copy link
Contributor Author

sbrugman commented Feb 29, 2024

The markup should either be added conditional on that the rich handler is used, or stripped afterwards.

Instead of including the markup directly in the log, it could be parametrized:

# Similar to `click`'s style function
def fmt(value: str, color: str):
    return f"[{color}]{value}[/{color}]"

...

# Configurable, inferred from the logging handler, or conditional on `rich` installed.
markup = False

  self._logger.info(
      "Loading data from %s (%s)...",
      markup(name, color="dark_orange") if markup else name,
      type(dataset).__name__,
      extra={"markup": markup},
  )

instead of

        self._logger.info(
            "Loading data from [dark_orange]%s[/dark_orange] (%s)...",
            name,
            type(dataset).__name__,
            extra={"markup": True},
        )

https://github.com/kedro-org/kedro/blob/main/kedro/io/data_catalog.py#L487

For my use case it would be enough to disable the markup if rich is not installed, but no strong preference.

Alternatively, you could include a NoRichHandler that strips out the formatting. This would be unintuitive (why need to do additional work for requiring fewer features), but it works.

Rich markup reference: https://rich.readthedocs.io/en/stable/markup.html

If we agree on an approach, I could submit a PR or perhaps @noklam can include it in #3657. There are not many places that need to be changed.

@noklam
Copy link
Contributor

noklam commented Feb 29, 2024

I think a simple condition is enough here, #3657 will not be merged soon feel free to open a PR @sbrugman.

@astrojuanlu
Copy link
Member

I assume this was fixed in #3682.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Logging Issue: Feature Request New feature or improvement to existing feature
Projects
Archived in project
Development

No branches or pull requests

3 participants