Support multiple spinners running in parellel

frostming commented 3 years ago

Description of the new feature, or changes

Add multi-threading support so that spinners can be updated individually without affecting other instances.

Example script:

import random
import time
from concurrent.futures import ThreadPoolExecutor

from halo import Halo

def long_running_task(i):
    with Halo(f"Running {i}", spinner="dots") as sp:
        time.sleep(3 + random.random())
    sp.succeed(f"Success {i}")

with ThreadPoolExecutor(max_workers=4) as pool:
    # Number of tasks is greater than max workers
    pool.map(long_running_task, range(8))

And here is the output

test

Checklist

[x] Your branch is up-to-date with the base branch
[x] You've included at least one test if this is a new feature
[x] All tests are passing

Related Issues and Discussions

Close #135 Close #17

halo_notebook is not updated. Tests are not supplied yet, any suggestions are welcome.

People to notify

frostming commented 3 years ago

/cc @adamtheturtle @manrajgrover @CoziestYew804

adamtheturtle commented 3 years ago

@frostming Thanks for mentioning me, and the gif looks great. I won't review as I don't have the expertise but I hope that this, or similar, makes it into halo!

manrajgrover commented 3 years ago

@frostming Thanks for working on this. It looks great!

I'll review this over the weekend. But this does require testing and support for notebook environment.

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 442

39 of 40 (97.5%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+1.5%) to 84.584%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
halo/halo.py	39	40	97.5%
<!--	Total:	39	40	97.5%	-->

Totals
Change from base Build 440:	1.5%
Covered Lines:	334
Relevant Lines:	388

💛 - Coveralls

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 441

38 of 39 (97.44%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+1.1%) to 84.146%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
halo/halo.py	38	39	97.44%
<!--	Total:	38	39	97.44%	-->

Totals
Change from base Build 440:	1.1%
Covered Lines:	333
Relevant Lines:	387

💛 - Coveralls

frostming commented 3 years ago

I added a test to cover the basic scenario of multiple running spinners.

But this does require testing and support for notebook environment.

Notebook should be working fine without any changes because instances are rendered in their own output widget, which I confirmed in a local notebook.

frostming commented 3 years ago

@frostming Thanks for mentioning me, and the gif looks great. I won't review as I don't have the expertise but I hope that this, or similar, makes it into halo!

Hi, Any progress on the review?

edgarcosta commented 3 years ago

For some reason on the notebook very often one of the widgets gets overwritten: halo

and in the console I see a lot of:

Popped wrong message (xxx instead of yyy) - likely the stack was not maintained in kernel.

phylroy commented 2 years ago

Any news on this being merged in? Looks like a nice addition.

mrkmndz commented 2 years ago

@manrajgrover is there any reason this PR hasn't been approved? Is this project actively maintained anymore?

edgarcosta commented 2 years ago

This feature is still broken on notebooks.

On Thu, Sep 23, 2021, 20:35 Mark Mendoza @.***> wrote:

@manrajgrover https://github.com/manrajgrover is there any reason this PR hasn't been approved? Is this project actively maintained anymore?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/manrajgrover/halo/pull/155#issuecomment-926259875, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACO2BRGEK67KCZT75YHJSTUDPBSJANCNFSM4TCBQIXQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

ostanislaw commented 2 years ago

Very good change. I also would like several spinners for multiprocessing.

black-snow commented 2 years ago

Still not merged? I was looking for exactly this. If anyone is reading this and has found an alternative to halo that works with multiple spinners, please ping me.

frostming commented 2 years ago

Still not merged? I was looking for exactly this. If anyone is reading this and has found an alternative to halo that works with multiple spinners, please ping me.

@black-snow I think this project is no longer maintained, have a look at rich you may achieve the same using rich's spinner and Live Display

nmacholl commented 1 year ago

@frostming thanks for the work you put into this PR; unfortunate the project isn't maintained.

manrajgrover / halo