-
Notifications
You must be signed in to change notification settings - Fork 910
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 rich optional / not a core dependency of kedro
#2928
Comments
kedro[rich]
to make rich not a core dependency of kedro
kedro[rich]
or make rich not a core dependency of kedro
Would be great to move this to a More broadly, configuring our logging is consistently a pain. This is hardly Kedro's fault, stdlib logging is famously obtuse, but our heavy handed approach to logging exacerbates this problem. Just from this week, a user is confused about logs or lack of them in Kedro, Docker, Airflow https://linen-slack.kedro.org/t/15975835/version-1-disable-existing-loggers-true-formatters-simple-fo#602b2164-1a4f-4e5c-8741-3c3dc29f7be7 An approach worth exploring is moving our logging to structlog https://www.structlog.org/en/stable/configuration.html (which has progressive enhancement if rich is present) and even consider making the library functions more logger-agnostic by passing it as a parameter in key places. |
rich created some problems in older versions of Databricks https://linen-slack.kedro.org/t/16022133/situation-kedro-0-18-8-python-3-8-16-databricks-9-1-lts-we-a#e098da1b-ef71-4ead-9044-d2e4dfe097d9 Possible solutions:
|
@datajoely 's kedro-rich prototype keeps receiving attention datajoely/kedro-rich#11 |
Unsure if this is a parent issue, because the linked issues are consequences of having rich as mandatory dependency, not a breakdown of tasks needed to get rid of it. I'm renaming accordingly. |
kedro[rich]
or make rich not a core dependency of kedro
kedro
Yeah aligned on the fact we should look to make the dependency optional Longer term I'm still keen to advocate for the other tasks still on the roadmap such as progress bars: |
And to do the drop in click replacement (which we could do a try/except import) |
Commenting here to clarify my view on this issue since it got mentioned in a meeting. Initially, my idea was that, for now, we should not change what users get when doing However, let's say we want to make the Kedro work even after doing
And, perhaps most importantly, even if we make So, I see 2 main blockers:
Thoughts @noklam @SajidAlamQB @lrcouto ? |
This is a good point, but I think it's a separate discussion from what we were talking.
Should we focus on 1. first, which is what this PR mainly addressing? So far I think the bigger problem coming from The 2. is still a problem, but in my mind a less urgent one until we have a good replacement of cookiecutter. Alternatively we can immediately pin |
Agreed to focus on (1) first 👍🏼 |
The PR in question is #3838 by the way |
@lrcouto is this completely addressed now or still follow up to do? |
There's still follow-up to do. What I implemented here was a first step, but it still requires the user to manually downgrade their To make |
@lrcouto in that case, would you prefer to reopen this issue or creates new one for the follow up tasks you mentioned? |
I'd say let's reopen this issue to signal it's not fully addressed, and we make it a parent of the new one about cookiecutter. |
Yeah, I agree. |
We will be closing this issue for now, based on this discussion on Kedro's dependencies. We have decided on a longer term approach to deal with Cookiecutter, and by extension Rich, that will take some time to design and implement. This issue will be addressed once we're ongoing with this process. |
Description
Documenting some conversation in Slack - we should consider making
rich
non-core of Kedro. PotentiallyWhile we have fixed many issues in this Tweaking Logging to improve UX, particular we introduce
RichHandler
. However, there are still some unresolved issues withrich
. Some of them requires upstream fix instead of kedro.List of unresolved issues, mostly IDE(notebook, VSCode or PyCharm) specific
rich
is mostly useful for interactive development workflow. In most production environment, teminal logs are not colored andrich
automatically fallback.Context
While Rich logging improve the readability, it has been proved hard to make it works 100% the case, there are edge cases in IDE/deployment environment which is out of our control. In some case, if the logging is too long
rich
automatically split it into multiple lines and cause issue.In addition, some user prefer plain logging (it's configurable, but user will still have to install rich)
Pro:
pip uninstall rich
)rich
optional to reduce Kedro's core dependenciesCon:
Note
This is not our top priority now, but we would love to have feedback and more discussion before we move to technical design or implementation.
Alternative
pip install kedro-rich
, not core dependencies) - https://github.com/datajoely/kedro-richThe text was updated successfully, but these errors were encountered: