-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[Data] Truncate progress bar description #46801
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,15 +44,22 @@ class ProgressBar: | |
because no tasks have finished yet), doesn't display the full | ||
progress bar. Still displays basic progress stats from tqdm.""" | ||
|
||
# If the name/description of the progress bar exceeds this length, | ||
# it will be truncated. | ||
MAX_NAME_LENGTH = 100 | ||
|
||
def __init__( | ||
self, | ||
name: str, | ||
total: Optional[int], | ||
unit: str, | ||
position: int = 0, | ||
enabled: Optional[bool] = None, | ||
display_full_name: bool = False, | ||
): | ||
self._desc = name | ||
# If True, disables name trunctating. | ||
self._display_full_name = display_full_name | ||
self._desc = self._truncate_name(name) | ||
self._progress = 0 | ||
# Prepend a space to the unit for better formatting. | ||
if unit[0] != " ": | ||
|
@@ -83,6 +90,11 @@ def __init__( | |
needs_warning = False | ||
self._bar = None | ||
|
||
def _truncate_name(self, name: str) -> str: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we add a warn-once that the name is getting truncated and that the behavior can be disabled with |
||
if not self._display_full_name and len(name) > self.MAX_NAME_LENGTH: | ||
return name[: self.MAX_NAME_LENGTH - 3] + "..." | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: (Don't need to do this, just throwing it out as an idea) I'm wondering if we should include some of the text at the end. So, for example, instead of:
We could do something like
Might make it clearer which operator it ends on. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah sounds good. now the updated output looks like:
|
||
return name | ||
|
||
def block_until_complete(self, remaining: List[ObjectRef]) -> None: | ||
t = threading.current_thread() | ||
while remaining: | ||
|
@@ -117,6 +129,7 @@ def fetch_until_complete(self, refs: List[ObjectRef]) -> List[Any]: | |
return [ref_to_result[ref] for ref in refs] | ||
|
||
def set_description(self, name: str) -> None: | ||
name = self._truncate_name(name) | ||
if self._bar and name != self._desc: | ||
self._desc = name | ||
self._bar.set_description(self._desc) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would this be set to True by the user? Is the expectation that they would not want that or should we have something in data context or execution options to allow for this configuration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yeah good point, i will need to expose it from DataContext