Since the figure attributes change from one version to another, we should possibly write plotly.js version to the output, IMO.

we should possibly write plotly.js version to the output, IMO.

Not a bad idea!

Should we write it side-by-side with the data and layout attributes or put it in layout.version?

Should we write it side-by-side with the data and layout attributes or put it in layout.version?

Shouldn't we also consider config? Regardless, I think it would make sense to include the version.

cc @alexcjohnson

Shouldn't we also consider config

We could, but in general the config won't be JSON-serializable. We could include only the JSON-serializable parts of the config and maybe add a placeholder for functions, for example:

modeBarButtonsToAdd: [{
  name: 'my button',
  click: function(gd) { console.log('hello') }
}]

// would become

{
  modeBarButtonsToAdd: [{
    name: 'my button',
    click: '_function_'
  }]
}

One more question:
Is the resulting object sorted by keys?

Yes, I'd include config using @etpinard's suggestion for functions. And very nice idea @archmoj re: version. I'd say it goes at the top level, to make it clear it's not actually an attribute.

@etpinard is it OK to assume nothing in data or layout is ever a function?

If that's the case, can I simply replace

        if(typeof d === 'function') {
            return '_function_';
        }

That should be safe and non-breaking, right?

is it OK to assume nothing in data or layout is ever a function?

No, there are plenty of functions inside fullLayout e.g. fullLayout.xaxis.l2p

plots.graphJson already removes attributes with leading underscores and values that are functions. That covers everything outside the schema that's in data and layout. config converting to gd._context is handled differently and as @etpinard points out has some functions in the actual user-provided values, so sending it back will need special handling as well.

I'm not sure about the value of returning config here TBH...

config could certainly be omitted at least to start - could always add it in later, perhaps if and when we convert it to using the same coerce machinery as data and layout.

@alexcjohnson adding config is not too bad (see 9497ed5)

The problem I have at the moment is outputting the version number (ie. Plotly.version) without getting circular dependency (see 9497ed5#r37388098). What's the easiest way to refer to the version number from anywhere in the code?

What's the easiest way to refer to the version number from anywhere in the code?

The trick is to hardcode it just like in

and

and then add a line below

updateVersion(/* path/to/file/ */);

and let the update_version.js util do the rest.

At the moment, when downloading the file via downloadImage we end up with a filename with extension .full-json. Should we instead set the extension to .full.json or .json?

cc @plotly/plotly_js @nicolaskruchten

I like .json but this is a weakly-held preference.

some TODOs here:

• resolve conflicts
• sort object keys
• use .json extension

• sort object keys

I'm wondering if sorting the object's keys recursively is really required? Would it be valuable to users?

cc @alexcjohnson @nicolaskruchten @archmoj

Sorting isn’t a hard requirement, could be omitted for now, but I do think it could be useful. @archmoj pointed out diffing, though I guess _full* will have a consistent order anyway due to the coerce process. But regardless, some objects will be pretty big, sorting will make it easier to find things.

Sorting isn’t a hard requirement, could be omitted for now, but I do think it could be useful.

Ok so would 81b8813 be satisfactory? I guess I could turn the forEach into a for loop. Any other comments?

Ok so would 81b8813 be satisfactory?

Looks great to me - I wouldn't worry about for vs forEach there, these are not looping over data arrays, and are not being called often.

No other comments from me, let's do it!