Allow passing TBPlugin/TBLoader subclasses to TensorBoardWSGIApp #2582
Conversation
tensorboard/backend/application.py
Outdated
| """Returns a plugin loader for the given plugin. | ||
| Args: | ||
| plugin: A TBPlugin subclass, or a TBLoader instance or subclass. |
There was a problem hiding this comment.
Maybe call this plugin_spec, as above, as it need not be a plugin?
tensorboard/backend/application.py
Outdated
| """ | ||
| if isinstance(plugin, base_plugin.TBLoader): | ||
| return plugin | ||
| if isinstance(plugin, type): |
There was a problem hiding this comment.
Just to make sure, this guard is just for readability, right?
There was a problem hiding this comment.
Without it you get an error for passing something that isn't a class to issubclass, actually.
tensorboard/program.py
Outdated
| :type plugins: list[Union[base_plugin.TBLoader, Type[base_plugin.TBPlugin]]] | ||
| :type plugins: list[Union[base_plugin.TBLoader, | ||
| Type[base_plugin.TBLoader], |
There was a problem hiding this comment.
nit: Can we try to not introduce brittle aligned-indents in favor of
hanging indents? Every time I run across one or two or three I cry
a little, and the far-right ones look awkward even when correctly
formatted.
There was a problem hiding this comment.
I just got rid of these :type lines, which was my original instinct since they're a pain to format legibly and we don't use them consistently or in an enforceable way.
3 participants
This is a follow up to #2576 that makes the new
TensorBoardWSGIApp()entry point accept types besides TBLoader instances for itspluginsargument, namely it also accepts aTBPluginsubclass (which it will wrap in aBasicLoader) or aTBLoadersubclass (which it will instantiate with no arguments to get an instance).I moved the existing helper in
program.TensorBoard()intoapplication.pyfor this, which means that API also has gained the ability to take aTBLoaderclass (and a better error if passing in invalid types).Lastly, I've streamlined the plugin list in
default.pyto just use bareTBLoadersubclasses, which liberates 10 parentheses to be used for nobler purposes.