[Data] Progress Reporting Refactor 3 by kyuds · Pull Request #59880 · ray-project/ray

kyuds · 2026-01-06T07:16:37Z

Description

Third PR for isolating progress managers / making them easier to work with.

Fully unify interfaces for all progress managers
Introduce the Noop progress --> basically to use for no-op situations
Separate out function for determining which progress manager to use
Add in verbose_progress setting for ExecutionOptions. This was missing from previous versions, mb

Related issues

N/A

Additional information

N/A

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

gemini-code-assist

Code Review

This pull request is a good refactoring of the progress reporting logic. It successfully unifies the interfaces for progress managers and centralizes the logic for selecting which manager to use into a new get_progress_manager function. The introduction of Noop progress managers for disabled cases is a clean and effective approach.

I've identified a couple of critical issues where the new verbose_progress parameter is not being passed to the progress manager constructors, which will lead to a TypeError. Additionally, I've pointed out some incorrect type hints in the tqdm progress manager that should be corrected for type safety and consistency.

After addressing these points, this will be a solid improvement to the codebase.

python/ray/data/_internal/progress/utils.py

python/ray/data/_internal/progress/tqdm_progress.py

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

python/ray/data/_internal/progress/utils.py

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

python/ray/data/_internal/progress/rich_progress.py

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

python/ray/data/_internal/progress/rich_progress.py

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

python/ray/data/_internal/progress/rich_progress.py

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

bveeramani · 2026-01-06T08:48:23Z

python/ray/data/_internal/progress/base_progress.py

        ...
+
+
+class NoopSubProgressBar(BaseProgressBar):


Nit: Should the Noop implementations go into its own module like the other concrete subclasses?

decision was made because 1) its short, like no logic 2) other progress managers also use the NoopSubProgressBar, so its easier to import the base. Open to moving it out if needed

SG. Not strongly opinionated

python/ray/data/_internal/execution/streaming_executor.py

python/ray/data/_internal/progress/utils.py

bveeramani · 2026-01-10T03:04:58Z

python/ray/data/_internal/progress/tqdm_progress.py

+
+            contains_sub_progress_bars = isinstance(op, SubProgressBarMixin)
+            sub_progress_bar_enabled = show_op_progress and (
+                contains_sub_progress_bars or verbose_progress
+            )
+
+            # create operator progress bar
+            uid = uuid.uuid4()
+            if sub_progress_bar_enabled:


This diff introduces a bug.

import time import ray ray.data.DataContext.get_current().execution_options.verbose_progress = False def sleep(row): time.sleep(1) return row ray.data.range(100).map(sleep).materialize()

On master, the progress bar shows the operator-level progress bars (expected), but on this PR, the operator-level progress bars are hidden.

Also, as I read the code, I felt confused about where we create the operator-level progress bars and where we create the operator subprogress bars. I think I felt confused because this diff treats operator-level progress bars as subprogress bars, but that doesn't match my understanding of the concepts

operator-level progress bars and operator subprogress bars are inherently different because the former is updated in streaming_executor while the latter is updated all over the place and passed around via TaskContext. I think you get confused with the tqdm implementation because the class TqdmSubProgressBar itself is reused for all tqdm-related progress bars.

for verbose_progress, as explained in later comment, this is fixing a feature that I accidentally broke before, so its more of a bug fix on current master branch vs. me creating a breaking behavior. Again, I am very sorry about this.

I see. TqdmSubProgressBar is both operator-level progress bar, as well as operator sub-progress bars.

Would it make sense to just call the class TqdmProgressBar then? The "Sub" suggested to me that it's related to the sub-progress bars

bveeramani · 2026-01-10T03:17:52Z

python/ray/data/_internal/progress/rich_progress.py

+        self._max_name_length = state.get("max_name_length", 100)


 class RichExecutionProgressManager(BaseExecutionProgressManager):


I think there's a similar bug here where we don't show operator progress if verbose isn't enabled

import time import ray ray.data.DataContext.get_current().execution_options.verbose_progress = False ray.data.DataContext.get_current().enable_rich_progress_bars = True ray.data.DataContext.get_current().use_ray_tqdm = False def sleep(row): time.sleep(1) return row ray.data.range(100).map(sleep, memory=1).materialize()

ok for this, we also discussed this offline on slack, but this was actually a bug that I missed out on a previous PR. This is the intended behavior that I said was going to reintroduce.

Oh, that's right. I think we should report operator-level progress even if verbose_progress=False, but I guess that's orthogonal to this PR...

think this is more of a product decision. If we want to ignore verbose_progress, we should just remove it to confuse extra handles. Relevant handles already include enable_operator_progress_bars and enable_progress_bars, so verbose_progress does seem like a duplicate. Willing to do a deeper dive into this if this direction seems positive. CC'ing @richardliaw @gvspraveen

Yeah. The current state is kind of a mess.

Open to proposals if you have ideas for how to clean this up

bveeramani · 2026-01-10T03:19:19Z

python/ray/data/_internal/progress/rich_progress.py

+                    _update_with_conditional_rate(progress, tid, metrics)
+            self._total.update(self._total_task_id, description=desc, **kwargs)
+            self.refresh()
+            time.sleep(0.02)


Out-of-scope for this PR, but I think this 0.02 magic number is hard to understand. Could you remind me what happens if we don't have it?

added comment explaining. TLDR: this delay is needed between rich Live refresh and stop so that the latest data is correctly reflected and rendered (before Live module is stopped)

Is there are more robust method to achieve this?

I honestly tried and failed on multiple attempts. Delay to print output does seem necessary. Will keep this on the back of my head though.

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

cursor · 2026-01-10T03:43:49Z

python/ray/data/_internal/progress/tqdm_progress.py

+
+            # create operator progress bar
+            uid = uuid.uuid4()
+            if sub_progress_bar_enabled:


Operator progress bars hidden when verbose_progress is disabled

High Severity

The condition sub_progress_bar_enabled = show_op_progress and (contains_sub_progress_bars or verbose_progress) incorrectly gates operator-level progress bar creation. When verbose_progress=False and an operator doesn't implement SubProgressBarMixin, operator-level progress bars are hidden even though show_op_progress is True. Previously, operator-level progress bars were shown whenever show_op_progress was True, regardless of these additional conditions. This causes a regression where standard operator progress bars disappear for most operators.

Additional Locations (1)

python/ray/data/_internal/progress/rich_progress.py#L184-L189

no this is intended behavior...

bveeramani

LGTM as an incremental improvement

bveeramani · 2026-01-12T08:22:40Z

python/ray/data/_internal/progress/base_progress.py

        ...
+
+
+class NoopSubProgressBar(BaseProgressBar):


SG. Not strongly opinionated

bveeramani · 2026-01-12T08:32:14Z

python/ray/data/_internal/progress/rich_progress.py

+                    _update_with_conditional_rate(progress, tid, metrics)
+            self._total.update(self._total_task_id, description=desc, **kwargs)
+            self.refresh()
+            time.sleep(0.02)


Is there are more robust method to achieve this?

bveeramani · 2026-01-12T08:32:58Z

python/ray/data/_internal/progress/rich_progress.py

+        self._max_name_length = state.get("max_name_length", 100)


 class RichExecutionProgressManager(BaseExecutionProgressManager):


Oh, that's right. I think we should report operator-level progress even if verbose_progress=False, but I guess that's orthogonal to this PR...

bveeramani · 2026-01-12T08:35:12Z

python/ray/data/_internal/progress/tqdm_progress.py

+
+            contains_sub_progress_bars = isinstance(op, SubProgressBarMixin)
+            sub_progress_bar_enabled = show_op_progress and (
+                contains_sub_progress_bars or verbose_progress
+            )
+
+            # create operator progress bar
+            uid = uuid.uuid4()
+            if sub_progress_bar_enabled:


I see. TqdmSubProgressBar is both operator-level progress bar, as well as operator sub-progress bars.

Would it make sense to just call the class TqdmProgressBar then? The "Sub" suggested to me that it's related to the sub-progress bars

## Description Third PR for isolating progress managers / making them easier to work with. - Fully unify interfaces for all progress managers - Introduce the `Noop` progress --> basically to use for no-op situations - Separate out function for determining which progress manager to use - Add in `verbose_progress` setting for `ExecutionOptions`. This was missing from previous versions, mb ## Related issues N/A ## Additional information N/A --------- Signed-off-by: Daniel Shin <kyuseung1016@gmail.com> Signed-off-by: jasonwrwang <jasonwrwang@tencent.com>

## Description Third PR for isolating progress managers / making them easier to work with. - Fully unify interfaces for all progress managers - Introduce the `Noop` progress --> basically to use for no-op situations - Separate out function for determining which progress manager to use - Add in `verbose_progress` setting for `ExecutionOptions`. This was missing from previous versions, mb ## Related issues N/A ## Additional information N/A --------- Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

## Description Third PR for isolating progress managers / making them easier to work with. - Fully unify interfaces for all progress managers - Introduce the `Noop` progress --> basically to use for no-op situations - Separate out function for determining which progress manager to use - Add in `verbose_progress` setting for `ExecutionOptions`. This was missing from previous versions, mb ## Related issues N/A ## Additional information N/A --------- Signed-off-by: Daniel Shin <kyuseung1016@gmail.com> Signed-off-by: peterxcli <peterxcli@gmail.com>

kyuds added 3 commits December 30, 2025 22:18

add noop progress manager

6135e03

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

consolidate progress manager usage

81be96d

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

Merge branch 'master' into progress-refactor-3

9d818eb

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

kyuds requested a review from a team as a code owner January 6, 2026 07:16

gemini-code-assist bot reviewed Jan 6, 2026

View reviewed changes

python/ray/data/_internal/progress/utils.py Outdated Show resolved Hide resolved

python/ray/data/_internal/progress/utils.py Outdated Show resolved Hide resolved

python/ray/data/_internal/progress/tqdm_progress.py Outdated Show resolved Hide resolved

kyuds added 3 commits January 5, 2026 21:19

fix bug from previous (non-merged) implementation

928b19a

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

add in verbose_progress flag to progress managers (bug fix)

d8c5e05

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

address gemini comment

48bf3c7

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

cursor bot reviewed Jan 6, 2026

View reviewed changes

python/ray/data/_internal/progress/utils.py Outdated Show resolved Hide resolved

kyuds added 2 commits January 5, 2026 21:44

show name for rich sub progress bar

03e9ded

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

noop importing bug fix

3982843

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

cursor bot reviewed Jan 6, 2026

View reviewed changes

python/ray/data/_internal/progress/rich_progress.py Show resolved Hide resolved

edge case for starttime initialization

9048b97

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

cursor bot reviewed Jan 6, 2026

View reviewed changes

python/ray/data/_internal/progress/rich_progress.py Outdated Show resolved Hide resolved

debug test

017450d

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

kyuds added the go add ONLY when ready to merge, run all tests label Jan 6, 2026

fix cursor bug

768975f

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

cursor bot reviewed Jan 6, 2026

View reviewed changes

python/ray/data/_internal/progress/rich_progress.py Show resolved Hide resolved

add state

54bf6e6

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

ray-gardener bot added data Ray Data-related issues community-contribution Contributed by the community labels Jan 6, 2026

bveeramani reviewed Jan 10, 2026

View reviewed changes

kyuds added 4 commits January 9, 2026 17:27

remove dead code

af86e0a

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

add comment explaining magic number

48fbb91

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

move get_progress_manager function to init

cefdfd5

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

merge master

f661d5e

Signed-off-by: Daniel Shin <kyuseung1016@gmail.com>

kyuds requested a review from bveeramani January 10, 2026 03:36

cursor bot reviewed Jan 10, 2026

View reviewed changes

bveeramani approved these changes Jan 12, 2026

View reviewed changes

bveeramani merged commit 1d22032 into ray-project:master Jan 12, 2026
6 checks passed

kyuds deleted the progress-refactor-3 branch January 12, 2026 09:10

		self._max_name_length = state.get("max_name_length", 100)


		class RichExecutionProgressManager(BaseExecutionProgressManager):

Conversation

kyuds commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Additional information

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyuds Jan 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor bot Jan 10, 2026

Choose a reason for hiding this comment

Operator progress bars hidden when verbose_progress is disabled

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bveeramani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kyuds commented Jan 6, 2026 •

edited

Loading

kyuds Jan 10, 2026 •

edited

Loading