Skip to content

Make notify-on-logging work with newPlot #4555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nicolaskruchten opened this issue Feb 6, 2020 · 8 comments
Open

Make notify-on-logging work with newPlot #4555

nicolaskruchten opened this issue Feb 6, 2020 · 8 comments
Labels
bug something broken P3 backlog sev-4 cosmetic

Comments

@nicolaskruchten
Copy link
Contributor

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

@nicolaskruchten nicolaskruchten added this to the v1.53.0 milestone Feb 6, 2020
@etpinard
Copy link
Contributor

etpinard commented Feb 6, 2020

.. and logging

@etpinard etpinard added the feature something new label Feb 6, 2020
@nicolaskruchten nicolaskruchten modified the milestones: v1.53.0, v1.54.0 Mar 5, 2020
@etpinard
Copy link
Contributor

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

var dfltConfig = require('../plot_api/plot_config').dfltConfig;

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:

function setPlotConfig(obj) {
return Lib.extendFlat(dfltConfig, obj);
}

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

function setPlotContext(gd, config) {
if(!gd._context) {
gd._context = Lib.extendDeep({}, dfltConfig);
// stash <base> href, used to make robust clipPath URLs
var base = d3.select('base');
gd._context._baseUrl = base.size() && base.attr('href') ?
window.location.href.split('#')[0] :
'';
}
var context = gd._context;
var i, keys, key;
if(config) {
keys = Object.keys(config);
for(i = 0; i < keys.length; i++) {
key = keys[i];
if(key === 'editable' || key === 'edits') continue;
if(key in context) {
if(key === 'setBackground' && config[key] === 'opaque') {
context[key] = opaqueSetBackground;
} else {
context[key] = config[key];
}
}
}
// map plot3dPixelRatio to plotGlPixelRatio for backward compatibility
if(config.plot3dPixelRatio && !context.plotGlPixelRatio) {
context.plotGlPixelRatio = context.plot3dPixelRatio;
}
// now deal with editable and edits - first editable overrides
// everything, then edits refines
var editable = config.editable;
if(editable !== undefined) {
// we're not going to *use* context.editable, we're only going to
// use context.edits... but keep it for the record
context.editable = editable;
keys = Object.keys(context.edits);
for(i = 0; i < keys.length; i++) {
context.edits[keys[i]] = editable;
}
}
if(config.edits) {
keys = Object.keys(config.edits);
for(i = 0; i < keys.length; i++) {
key = keys[i];
if(key in context.edits) {
context.edits[key] = config.edits[key];
}
}
}
// not part of the user-facing config options
context._exportedPlot = config._exportedPlot;
}
// staticPlot forces a bunch of others:
if(context.staticPlot) {
context.editable = false;
context.edits = {};
context.autosizable = false;
context.scrollZoom = false;
context.doubleClick = false;
context.showTips = false;
context.showLink = false;
context.displayModeBar = false;
}
// make sure hover-only devices have mode bar visible
if(context.displayModeBar === 'hover' && !hasHover) {
context.displayModeBar = true;
}
// default and fallback for setBackground
if(context.setBackground === 'transparent' || typeof context.setBackground !== 'function') {
context.setBackground = setBackground;
}
// Check if gd has a specified widht/height to begin with
context._hasZeroHeight = context._hasZeroHeight || gd.clientHeight === 0;
context._hasZeroWidth = context._hasZeroWidth || gd.clientWidth === 0;
// fill context._scrollZoom helper to help manage scrollZoom flaglist
var szIn = context.scrollZoom;
var szOut = context._scrollZoom = {};
if(szIn === true) {
szOut.cartesian = 1;
szOut.gl3d = 1;
szOut.geo = 1;
szOut.mapbox = 1;
} else if(typeof szIn === 'string') {
var parts = szIn.split('+');
for(i = 0; i < parts.length; i++) {
szOut[parts[i]] = 1;
}
} else if(szIn !== false) {
szOut.gl3d = 1;
szOut.geo = 1;
szOut.mapbox = 1;
}
}

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:

var dfltConfig = require('../plot_api/plot_config').dfltConfig;

if(gd.undoQueue.queue.length > dfltConfig.queueLength) {
gd.undoQueue.queue.shift();
gd.undoQueue.index--;
}

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

@archmoj
Copy link
Contributor

archmoj commented Jul 3, 2020

Hmm... it seems we need to pass gd as an argument to these two API functions

exports.makeTemplate = function(figure) {

exports.validateTemplate = function(figureIn, template) {

which may require publishing a major version.

@archmoj
Copy link
Contributor

archmoj commented Jul 3, 2020

Hmm... it seems we need to pass gd as an argument to these two API functions

exports.makeTemplate = function(figure) {

exports.validateTemplate = function(figureIn, template) {

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 🤞

@archmoj
Copy link
Contributor

archmoj commented Jul 6, 2020

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

@nicolaskruchten
Copy link
Contributor Author

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

@gvwilson
Copy link
Contributor

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

@archmoj
Copy link
Contributor

archmoj commented May 27, 2024

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

It is still relevant.

@gvwilson gvwilson added sev-4 cosmetic P3 backlog and removed status: on hold labels Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken P3 backlog sev-4 cosmetic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants