Knowledgator / GLiClass

Generalist and Lightweight Model for Text Classification
33 stars 3 forks source link

Removed tqdm from pipeline.py #4

Closed whitelotusapps closed 1 month ago

whitelotusapps commented 1 month ago

In my own code, I am using tqdm and I noticed that whenever I was using ZeroShotClassificationPipeline, my tqdm progress bars would exhibit an annoying behavior of each iteration appearing on a new line. After reviewing my code, and attempting many ways to suppress this behavior, including trying another progress bar package (alive-progress) - I finally took a look at the GLiClass code. Upon review, I found that tqdm was also being used in the pipline.py code. Once I removed the usage of tqdm from the pipeline.py file on my local machine and the behavior of tqdm within my own code returned to normal.

Using tqdm within the library code itself will likely cause unintended issues with downstream usage that may also be using tqdm.

Regarding the removal of numpy and GLiClassModelConfig also appears to be inconsequential from my testing.

Thanks!

Ingvarstep commented 1 month ago

@whitelotusapps , thank you for trying our framework and sharing your opinion regarding tqdm inside the GLiClass pipeline. I think it will be better for users to have a parameter that will disable or enable the usage of tqdm inside the pipeline. What do you think?

whitelotusapps commented 1 month ago

Absolutely! I am enjoying the models, and I appreciate all of your work and contributions. Also, a parameter to enable/disable the progress bar within the library would be great; I hadn't thought of that. 😅🙏

Ingvarstep commented 1 month ago

Absolutely! I am enjoying the models, and I appreciate all of your work and contributions. Also, a parameter to enable/disable the progress bar within the library would be great; I hadn't thought of that. 😅🙏

Would you like to implement it?

whitelotusapps commented 1 month ago

To be honest, this is my first PR ever. I have updated the code to implement the progress bar as a parameter as shown in the example below:

ZeroShotClassificationPipeline(model, tokenizer, classification_type='multi-label', progress_bar=False)

The default value is to have progress_bar=True.

The multiple commits were due to my naivety on best practices, and my unsuccessful attempts as rebasing the commits into one cohesive commit. 😅

Ingvarstep commented 1 month ago

To be honest, this is my first PR ever. I have updated the code to implement the progress bar as a parameter as shown in the example below:

ZeroShotClassificationPipeline(model, tokenizer, classification_type='multi-label', progress_bar=False)

The default value is to have progress_bar=True.

The multiple commits were due to my naivety on best practices, and my unsuccessful attempts as rebasing the commits into one cohesive commit. 😅

No worries, and congrats with the first successful pull requests :-) For me, your realization looks good.

whitelotusapps commented 1 month ago

Awesome, thanks Ihor!! 🙏