Make notify-on-logging work with newPlot

[nicolaskruchten]

Right now we can't use this function from Python because we only set config during newPlot.

[etpinard]

.. and logging

[etpinard]

Currently, the loggers.js module requires the default plot config object:

https://github.com/plotly/plotly.js/blob/8f049fddbac0ca0382816984b8526857e9714fe6/src/lib/loggers.js#L13

and uses it to determine which log to show in the console and/or the notifier popups.

The default plot config object can be mutated using Plotly.setPlotConfig:

https://github.com/plotly/plotly.js/blob/8f049fddbac0ca0382816984b8526857e9714fe6/src/plot_api/plot_api.js#L402-L404

and this is the only way to get the logging and notifyOnLogging config option to work at the moment.

Routines that depend on the other config options do not rely on the default plot config object; they instead rely on the "graph" config (i.e context) coerced in gd._context during

https://github.com/plotly/plotly.js/blob/8f049fddbac0ca0382816984b8526857e9714fe6/src/plot_api/plot_api.js#L419-L524

So, to make the loggers.js know about gd._context, we'll need to pass gd to all Lib.log, Lib.warn and Lib.error calls. For example,

// on https://github.com/plotly/plotly.js/blob/8f049fddbac0ca0382816984b8526857e9714fe6/src/plot_api/plot_api.js#L146
Lib.log('Legacy polar charts are deprecated!');

// would become
Lib.log(gd, 'Legacy polar charts are deprecated!');

Things might get annoying for cases when gd isn't part of the scope in which Lib.(log|warn|error) is called from. Perhaps to make things easier to pass around, we could copy the logging and notifyOnLogging keys in fullLayout.

---

Note also, that queue module also uses the default plot config object:

https://github.com/plotly/plotly.js/blob/8f049fddbac0ca0382816984b8526857e9714fe6/src/lib/queue.js#L12

https://github.com/plotly/plotly.js/blob/8f049fddbac0ca0382816984b8526857e9714fe6/src/lib/queue.js#L91-L94

This thing is mostly deprecated, but maybe it would be a good idea to make it work with gd._context as well.

[archmoj]

Hmm... it seems we need to pass gd as an argument to these two API functions https://github.com/plotly/plotly.js/blob/12303db490e1fcd2efdc645c72d603ada1261764/src/plot_api/template_api.js#L33 https://github.com/plotly/plotly.js/blob/12303db490e1fcd2efdc645c72d603ada1261764/src/plot_api/template_api.js#L287 which may require publishing a major version.

[archmoj]

Hmm... it seems we need to pass gd as an argument to these two API functions https://github.com/plotly/plotly.js/blob/12303db490e1fcd2efdc645c72d603ada1261764/src/plot_api/template_api.js#L33

https://github.com/plotly/plotly.js/blob/12303db490e1fcd2efdc645c72d603ada1261764/src/plot_api/template_api.js#L287

which may require publishing a major version.

Support for graph div as first argument for Plotly.makeTemplate and Plotly.validateTemplate is added in #3111 and #3118. So there shouldn't be a need for v2 :crossed_fingers:

[archmoj]

@alexcjohnson @nicolaskruchten what should be added in terms of the API? New layout attributes?

[nicolaskruchten]

We already have a config option for this, so all that's needed is for it to work when set inline with newPlot :)

[gvwilson]

is this one still relevant or can we close it? thanks

[archmoj]

is this one still relevant or can we close it? thanks

It is still relevant.