Update logger.py to reset color escape sequence in between values

[d7919]

Addresses issue #488 by resetting colour code after each value in logger::_step.

Also resets the use of red in target_space when warning about duplicate points being added.

[d7919]

Hi @till-m,

Thanks for the review. Replacing Fore.BLACK with Fore.RESET is a good idea as a "safe" default.

I do think having a way to disable entirely could be useful for saving the screen logger output to file as well as to allow the user to avoid issues should they be running in a terminal with a magenta background (if there's anyone that does this!).

Currently there's no user exposure of the logger (created in _prime_subscriptions) . If this creation was moved to near the end of BayesianOptimization::__init__ and stored as self.logger it should provide some exposure. Then by providing a method on ScreenLogger of the form:

def disable_colors(self):
     self.NEW_MAX_COLOR = ''
     self.DEFAULT_COLOR=''
     self.RESET_COLOR=''

and changing the uses of Fore to NEW_MAX_COLOR etc. (which would be initialised in the __init__ to the current values from Fore) then there's a simple method to strip out all escape codes and a way for users to customise these if they really want.

This does feel like it might be exposing / adding a bit much -- I'm not really a python developer so there may be neater ways to do this, but I thought exposing the logger might be nicer than doing it all through init arguments which get passed down the call chain.

[till-m]

Hi @d7919,

I'm not really an expert regarding the Observer pattern that's in use here, so I can't really see a nice & clean way of handling things. It can be handled however:

from bayes_opt import BayesianOptimization
from bayes_opt.logger import ScreenLogger
from bayes_opt.event import Events

def black_box_function(x, y):
    return -x ** 2 - (y - 1) ** 2 + 1

pbounds = {'x': (2, 4), 'y': (-3, 3)}

optimizer = BayesianOptimization(
    f=black_box_function,
    pbounds=pbounds,
    verbose=2, # verbose = 1 prints only when a maximum is observed, verbose = 0 is silent
    random_state=1,
)

logger = ScreenLogger()
logger._default_cell_size = 15 # changing something to show that it works

# the following needs to happen before any call to `.maximize`
# otherwise there will be two loggers associated with the optimizer.
for e in [Events.OPTIMIZATION_START, Events.OPTIMIZATION_STEP, Events.OPTIMIZATION_END]:
    optimizer.subscribe(e, logger)

optimizer.maximize(
    init_points=2,
    n_iter=3,
)

This does feel like it might be exposing / adding a bit much

IMO as long as things are handled by the ScreenLogger and abstracted from the BayesianOptimization class it would be fine.

Let me know how you wish to handle things. I'm fine with merging this PR as-is, but I would also be open to adding the options discussed above.

[d7919]

Hi @till-m,

Maybe best to leave this PR as-is and leave any further modification of colourising output to the future.

I think the appearance of colours outside of the logger would suggest that to handle customisation of this most consistently probably requires a slight redesign (perhaps a simple container class through which colour escape codes can be obtained). There's also the fact that BayesianOptimization takes and stores verbose but only uses this to pass through to _get_default_logger which may suggest a slight refactor might help separate concerns more completely?

Ultimately this isn't something that really impacts on the utility of the code so no need to rush further changes in.

[till-m]

I agree, I think logging might be due for an overhaul at some point, though it is not a priority.

I would still argue that replacing Fore.BLACK with Fore.RESET would be a good change for now. Are you interested in handling that with this PR? Otherwise I can make a seperate PR.

[d7919]

I'm certainly happy to do that here. If that is being done here then how about the following:

class ScreenLogger(_Tracker):                                                                                                                                                                 
....                                                                                                                                                                                              
    _default_cell_size = 9                                                                                                                                                                    
    _default_precision = 4                                                                                                                                                                    
    _colour_new_max = Fore.MAGENTA                                                                                                                                                             
    _colour_regular_message = Fore.RESET                                                                                                                                                       
    _colour_reset = Fore.RESET                   

    def _step(self, instance, colour=_colour_regular_message):
....
        return "| " + " | ".join([colour + cells[i] + self._colour_reset for i in range(len(cells))]) + " |"
....

        elif event == Events.OPTIMIZATION_STEP:                                                                                                                                               
            is_new_max = self._is_new_max(instance)                                                                                                                                           
            if self._verbose == 1 and not is_new_max:                                                                                                                                         
                line = ""                                                                                                                                                                     
            else:                                                                                                                                                                             
                colour = self._colour_new_max if is_new_max else self._colour_regular_message                                                                                                       
                line = self._step(instance, colour=colour) + "\n"                                   

This way users that really want to customise this can in the way you showed but I don't worry about changing how this is exposed?

[till-m]

Looks good to me!

[till-m]

thanks for your contribution @d7919!