Plotly standard now fully supported #79
Conversation
python_samples.py
Outdated
| @@ -0,0 +1,75 @@ | |||
| from plotly.offline import download_plotlyjs, init_notebook_mode, plot, iplot | |||
There was a problem hiding this comment.
can you move this to a /test/samples/ folder?
src-backend/interpreter.ts
Outdated
| else{ | ||
| output = data[chosenType]; | ||
| } | ||
| if(typeof output === 'string'){ |
There was a problem hiding this comment.
this check is not needed
src-backend/interpreter.ts
Outdated
| @@ -146,10 +143,39 @@ export class ContentHelpers{ | |||
| } | |||
| } | |||
|
|
|||
| static interpretRich(data){ | |||
| let chosenType = this.chooseTypeFromComplexData(data); | |||
| let output = ''; | |||
There was a problem hiding this comment.
set the type of output to string
src-backend/interpreter.ts
Outdated
| @@ -76,6 +76,7 @@ export class ContentHelpers{ | |||
| static sourceTmp = ''; | |||
| static contentTmp: Array<CardOutput> = []; | |||
| static id = 0; | |||
| static contentId = 0; | |||
There was a problem hiding this comment.
this will introduce a lot of bugs later on. better to generate a GUID for every new element, which is what plotly does when showing text/html outputs. See https://stackoverflow.com/questions/105034/create-guid-uuid-in-javascript for inspiration.
src-backend/interpreter.ts
Outdated
| @@ -146,10 +143,39 @@ export class ContentHelpers{ | |||
| } | |||
| } | |||
|
|
|||
| static interpretRich(data){ | |||
There was a problem hiding this comment.
this function should return a CardOutput object instead of adding it directly to contentTmp.
src-backend/interpreter.ts
Outdated
| let traceback = (content['traceback'] as string[]).join('\n'); | ||
| this.contentTmp.push(new CardOutput('error', traceback)); | ||
| } | ||
| } | ||
|
|
||
| static interpretRich(data){ | ||
| static interpretRich(data: JSONValue){ |
src-backend/interpreter.ts
Outdated
|
|
||
| if(chosenType === 'application/vnd.plotly.v1+json'){ | ||
| let plotlyJson = data[chosenType]; | ||
| if(ContentHelpers.validateData(plotlyJson, 'data')){ | ||
| let guid = this.generateGuid() |
src-backend/interpreter.ts
Outdated
| } | ||
| } | ||
|
|
||
| static installMissingModule(data, module: string){ |
There was a problem hiding this comment.
It's the choice of the user. Whether to install or not. That's what it's usually used.
There was a problem hiding this comment.
it's not being used in the body of the function. besides, the check should probably be moved to the callback on line 174.
src-backend/interpreter.ts
Outdated
| if (module) { | ||
| let terminal = vscode.window.createTerminal('pip'); | ||
| terminal.show(); | ||
| terminal.sendText('pip install '+module, true); |
There was a problem hiding this comment.
shouldn't this be put into userInteraction?
There was a problem hiding this comment.
The terminal doesn't require any user interaction
src-backend/interpreter.ts
Outdated
|
|
||
| static getMissingModule(evalue: string){ | ||
| if(evalue.match(/No module named/i)){ | ||
| let moduleMatch = /'.+'/i.exec(evalue); |
There was a problem hiding this comment.
these two regexes can be combined into a single one, something like /No module named '(.+)'/i, and the name can be extracted with match[1]
src-backend/interpreter.ts
Outdated
| if(moduleMatch && moduleMatch.length){ | ||
| let module = moduleMatch[0].replace(/\'/g, ''); | ||
| vscode.window.showInformationMessage('Jupyter requires the module ' + moduleMatch[0] + ' to be installed. Install now?', 'Install') | ||
| .then(data => this.installMissingModule(data, module)); |
There was a problem hiding this comment.
shouldn't this be put into userInteraction?
There was a problem hiding this comment.
It's a built in function. Wouldn't make any sense calling the same thing in a sibling class. Interpreter should not have access to user interaction
There was a problem hiding this comment.
Unless you want an event? Wouldn't it be overkill?
There was a problem hiding this comment.
ok, fair enough. it's fine
Check out the python_samples.py file to test 3d plotting! :-)
Closes #67
If a module is missing the user is prompted to install it
Closes #62