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

Remove all output mgmt from the resolver #2060

Merged
merged 5 commits into from
Jul 31, 2024
Merged

Conversation

erikbern
Copy link
Contributor

@erikbern erikbern commented Jul 30, 2024

Instead of storing a Rich Tree on the resolver, create it on the output manager.

For convenience reasons it's useful to construct an insivisble tree if output isn't enabled. That way everywhere we add status rows, we don't have to check if output is enabled. However this means we need to use a class variable since the singleton may not exist.

We might want to change this in the future if we don't want Rich to be a dependency. It's not, hard just need to add a bunch of if statements.

@erikbern erikbern force-pushed the erikbern/resolver-no-output branch from aa7504d to 393a4f7 Compare July 30, 2024 00:22
@erikbern erikbern force-pushed the erikbern/resolver-no-output branch from d03897d to 5d5e301 Compare July 30, 2024 01:34
@erikbern erikbern changed the title (WIP) Remove all output mgmt from the resolver Remove all output mgmt from the resolver Jul 30, 2024
@erikbern erikbern requested a review from mwaskom July 30, 2024 15:05
@erikbern erikbern merged commit f1140b1 into main Jul 31, 2024
27 checks passed
@erikbern erikbern deleted the erikbern/resolver-no-output branch July 31, 2024 12:21
@erikbern
Copy link
Contributor Author

danke @mwaskom

erikbern added a commit that referenced this pull request Jul 31, 2024
erikbern added a commit that referenced this pull request Jul 31, 2024
* Revert "Fix issue with global output manager tree (#2062)"

This reverts commit 2346aa9.

* Revert "Remove all output mgmt from the resolver (#2060)"

This reverts commit f1140b1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants