Skip to content

Conversation

@wblachowski
Copy link

Hi, first of all - a great utility library, it's been a great time saver.
I'd like to suggest a small change that would allow the user to silent all prints as I found it a tad annoying to have the library spam my standard output with numerous messages, especially when polling for processing results.

I wondered whether some kind of a "global" silencing (passing a silent option once, in the VideoIndexer constructor), or a more seamless solution (such as overwriting sys.stdout) would be better, but in the end, I settled for the simplest solution, which is the ability to pass a silent arg to each method capable of printing something.

@bklim5
Copy link
Owner

bklim5 commented Aug 3, 2021

hey @wblachowski , thanks for the PR! I think that's a good idea. Maybe another suggestion from me, we can set the silent as the class variable, and have another function that we can use to replace all the existing print statements, otherwise, we have to do the if check every time we have a new print statement. Something like below:

class VideoIndexer():
    def __init__(self, silent):
        self.silent = silent

    def print_line(self, msg):
        if not self.silent:
            print(msg)
    
    def upload_to_video_indexer(self):
        print_line('Uploading video to video indexer...')
        ....

    def get_video_info(self):
        print_line('Getting video info for ....')
        ....

EDIT: oh wait, I think setting the silent in the constructor is what you suggested :D

@wblachowski
Copy link
Author

wblachowski commented Aug 3, 2021

Hi,
Yup, that's very similar to what I've considered initially. Frankly, I find the constructor-based solution more elegant with the only drawback of being less flexible. One example I had in mind was silencing the get_video_info() method only since it's the one I call repeatedly. At the same time, messages from other methods are more informative, for instance, I'd like upload_to_video_indexer() to still print any errors it encounters (most importantly of the current video being already processed by VI).

Yet another solution would be to pass a flag like NONE/INFO/DEBUG to the constructor and classify the messages accordingly, with NONE suppressing all messages, INFO printing the most important ones only (like errors), DEBUG printing everything.

class VideoIndexer():
    def __init__(self, log_level):
        self.log_level= log_level

    def print_info(self, msg):
        # Prints for INFO and DEBUG
        if self.log_level != LogLevel.NONE:
            print(msg)
            
    def print_debug(self, msg):
       # Prints for DEBUG only (too specific for INFO)
       if self.log_level == LogLevel.DEBUG:
           print(msg)
    
    def upload_to_video_indexer(self):
        print_info('Uploading video to video indexer...')
        ....

    def get_video_info(self):
        print_debug('Getting video info for ....')
        ....

It's somewhere in the middle between the two previous solutions.

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants