Do not violently remove config_dir kwarg of config manager.
#899
Conversation
Pr jupyter#882 removed `config_dir` kwarg as it was ignored. though, jupyter#893 argues that we might need it and that it will be complex to reintroduce it. This use a convoluted way to : - warn that this kwarg might not be given to config managers in future version - but do not warn on default installs with default config. - warn that the keyword is ignored when people use subclasses of out config manager, and pass the keyword to super(). Thus this actually advertise that the keyword **might** be removed, by still allowing us to keep it if in the end it appears that we need it. Should help with jupyter#893 without un-fixing jupyter#882.
It was remove primarily because it was "forcing" the
We don't need the My believe is that the |
| self.config_manager = self.config_manager_class( | ||
| parent=self, | ||
| log=self.log, | ||
| config_dir=os.path.join(self.config_dir, 'nbconfig'), |
There was a problem hiding this comment.
This is the whole thing... we don't need the config_dir from the notebookapp for the custom ConfigManager... this is my simple implementation of a CustomConfig manager: https://github.com/Anaconda-Server/nb_config_manager/blob/master/nb_config_manager/nb_config_manager.py
Since the config_dir is "calculated" inside the ConfigManager class (by the _config_dir_default), my subclass take that info and make further modifications from there... so, we are happy with just the if branch above, where the config_dir is not harcoded at the notebook level... maybe I am not clear enough in my explanations... sorry about that.
Check my comment in #893 (comment), with that diff you keep the things as now (I mean, not config option at the ConfigManager and the default config pointing to the user space), but you "hardcode" the config_dir at the ConfigManager level, so we can then subclass and just setup the config_dir to our desires without touching the notebookapp at all...
Pr #882 removed
config_dirkwarg as it was ignored.Though, #893 argues that we might need it and that it will be complex to
reintroduce it.
This use a convoluted way to :
config manager, and pass the keyword to super().
Thus this actually advertise that the keyword might be removed,
by still allowing us to keep it if in the end it appears that we need
it.
Should help with #893 without un-fixing #882.
It annoys me to go to that much convolution and weird things that close to final release, but if it's the cost of actually moving forward so be it.
Would that please both @ellisonbg and @damianavila ?