Closed thebigmunch closed 4 years ago
Actually yes. I was thinking about progress bars. Something like https://github.com/tqdm/tqdm but "prettier".
I was thinking of rendering them like this. Fairly minimal.
Are you thinking about having a bar
equivalent to box
to style them?
Are you thinking about having optional progress text? Maybe just take a callable that accepts whatever progress object you're working with and returns a string. One of the problems I've run into with tqdm's progress text is that it's difficult to customize in many ways because it's too specific. So, I think this would be a better way to do it.
Yeah, that seems like a good idea. In Rich terms it would be a 'renderable' so you could substitute it for anything that renders progress.
Loving the progress (pun intended) on this so far. Looking like the kind of simple progress bar UX I've wanted.
Thanks. API is coming together. Posted a example https://twitter.com/willmcgugan/status/1236715446197510144
Thanks. API is coming together. Posted a example twitter.com/willmcgugan/status/1236715446197510144
I've been moving over torrent CLI utility thorod over to rich. Already been playing with rich.progress to replace the bar I display when hashing files on torrent creation. Previously used tqdm.
Nice. Let me know how that goes. But bear in mind the interface in master may change before the next release.
Nice. Let me know how that goes. But bear in mind the interface in master may change before the next release.
Yeah, I've already had to change it once. But, getting most the work done and just having to change it a bit at the end is nice when you have limited time.
Here's a screen recording of a run of thorod with a progress bar to give you an idea of some of the things I might like to include in this particular bar: progress, rate, and elapsed time. I had to hack some things a bit to get there:
@dataclass
class HashingTask(Task):
start: pendulum.datetime = pendulum.now()
@property
def elapsed(self):
return humanize_duration((pendulum.now() - self.start).seconds)
@property
def progress(self):
return f"{humanize_filesize(self.completed)}/{humanize_filesize(self.total)}"
@property
def rate(self):
return f"{humanize_filesize(self.completed / (pendulum.now() - self.start).seconds)}/s"
class HashingProgress(Progress):
def add_task(
self,
name,
total: int = 100,
completed: int = 0,
visible: bool = True,
**fields: str
):
with self._lock:
task = HashingTask(name, total, completed, visible=visible, fields=fields)
self._tasks[self._task_index] = task
self.refresh()
try:
return self._task_index
finally:
self._task_index = TaskID(int(self._task_index) + 1)
Nice. You may be able to do that without the customized Progress class. The Progress constructor takes a list of columns, which can be a format string or a callable. The callable should take a Task object and return something Rich can render. Have a look at bar_widget
which works that way.
I'll probably implement widgets for ETA, file speed, etc. Similar to your properties.
BTW I think your start
variable will set the default once at import time. You may need field(default_factory=pendulum.now)
BTW I think your
start
variable will set the default once at import time. You may needfield(default_factory=pendulum.now)
Yeah. It's a CLI app, so it's one use and wasn't so critical when I was just testing things.
One thing that did bug me is that the add_task
method is a misnomer. I would expect it to add a Task
instance. But, what it does is create a Task
instance and then add it.
Nice. You may be able to do that without the customized Progress class. The Progress constructor takes a list of columns, which can be a format string or a callable. The callable should take a Task object and return something Rich can render. Have a look at
bar_widget
which works that way.
I had started playing around with using a customized bar like this. I found this to be a bit clunky, though. You can't just add an instance due to it being called in the code, hence the bar_widget
function to return an instance.
I also don't see how adding a Task
in the Progress
instantiation would work. Maybe I'm missing something.
Oh, I also noticed that Progress.update
takes **fields
, but then also doesn't do anything with them.
Updated my properties to handle zero-division and added a new one for an estimated time remaining:
@property
def progress(self):
return f"{humanize_filesize(self.completed, precision=2)}/{humanize_filesize(self.total, precision=2)}"
@property
def rate(self):
elapsed = (pendulum.now() - self.start).seconds
return "0 B/s" if elapsed == 0 else f"{humanize_filesize(self.completed / elapsed)}/s"
@property
def remaining_time(self):
elapsed = (pendulum.now() - self.start).seconds
if (
self.completed == 0
or elapsed == 0
):
remaining_time = "..."
else:
remaining_time = (
f"{humanize_duration((self.total - self.completed) / (self.completed / elapsed))} left"
)
return remaining_time
One thing that did bug me is that the add_task method is a misnomer. I would expect it to add a Task instance. But, what it does is create a Task instance and then add it.
Were you expecting it to accept a Task
instance? The task object was not really something I was intending to been customized. It's purely an internal detail.
I had started playing around with using a customized bar like this. I found this to be a bit clunky, though. You can't just add an instance due to it being called in the code, hence the bar_widget function to return an instance.
Not sure I follow. What do you find clunky? It's a one line function for the bar.
def bar_widget(task: Task) -> Bar:
"""Gets a progress bar widget for a task."""
return Bar(total=task.total, completed=task.completed, width=40)
Oh, I also noticed that Progress.update takes **fields, but then also doesn't do anything with them.
You can refer to the fields dict in a column format string. See downloader.py for an example.
Were you expecting it to accept a
Task
instance? The task object was not really something I was intending to been customized. It's purely an internal detail.
Yes. The wording add_task
is clear to me that it should add a task, not create one. The Task
class is external to Progress
and there are methods for updating and removing tasks. It's a bit weird that none of them actually work on a Task
. While you may have intended it to be an internal detail, it shines right through. When I looked at the code, it definitely seemed to not be an internal detail. Maybe that's just me.
Not sure I follow. What do you find clunky? It's a one line function for the bar.
That's what's clunky about it. I would expect to be able to pass a Bar
instance, for example. It's certainly not the worst thing in the world, but it is an oddity. I'd at least expect for bar_widget
to take the arguments to Bar
and be a way for users to create their own bar widget with that instead of having to create their own function in every project if they don't want a default bar.
You can refer to the fields dict in a column format string. See downloader.py for an example.
I'm aware. I already showed doing that in this issue. My point was that you can't update arbitrary fields on the Task
instance, even though you can set them. That's one of the reasons I went with a Task
subclass. For example, elapsed
could have been implemented this way instead of customizing a Task
.
I'm aware. I already showed doing that in this issue. My point was that you can't update arbitrary fields on the
Task
instance, even though you can set them.
I see that's changed now.
About Task
being an internal detail, I don't see how that can be. Users need to know about it and it properties to be able to use them as columns. They need to know that they can add and use fields to Task
classes. It is definitely not just an internal detail.
I'll say this about the *_task
method naming and usage:
list.remove
works on values/objects, not index.
list.pop
and dict.pop
work on key or indexcreate_task
or spawn_task
is more appropriate for what the add_task
currently does.add_task
completely mimics the Task
constructor.
This is a pretty big red flag that it should just accept a Task
instance.Task
should be part of the public API (which I think it already is), as subclassing it can lead to flexibility for users to implement things so rich doesn't have to.I'm on break from this. My social anxiety disorder is kicking my ass as it does in these kinds of situations. I can't justify doing anymore with rich unless markdown-like markup gets removed anyway. Thanks for your time and effort.
Let wait until I've finished working on it, and added documentation. It might me more clear what I intend to do with it. Have a look once you are feeling better.
Any plans/thoughts on including progress bar functionality?
I know they're another big pill to swallow, but they certainly could fit the premise of rich. Integrating outside progress bar libraries isn't great as it is right now.