Add Python Interactive Window by rchiodo · Pull Request #3034 · microsoft/vscode-python

rchiodo · 2018-10-26T20:29:24Z

Implements the Python Interactive window and associated commands to run python files as notebooks

Fix #2302

Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
Title summarizes what is changing
Has a news entry file (remember to thank yourself!)
Unit tests & system/integration tests are added/updated
package-lock.json has been regenerated by running npm install (if dependencies have changed)

…ienceChanges merge in changes from the old data science branch

…mUpdate

Tie this into vscode:prepublish Have normal build still build react parts Have normal hygiene still work on react parts

…amUpdate Upstream update and CI test

…view_support

…webview_support Adding building and hygiene support for react webview controls

…rtup (#2711) * Fixup naming inconsistencies * Add LanguageServerFolderService mock to other activationService tests.

For #2351

* Dispatch on-type formatters to prevent them from overriding each other * Move providers out of dispatcher, write tests * Add news entry

* Remove reporting of ls version * Remove file

For #2723

We adjust to changes in 4.4.0.

…ngs.json (#2750) * Prefix relative paths with './' * Ensure relative path is prefixed with ./

I'm seeing compiler errors that have slipped in over the last few weeks. This PR fixes them. All but one are skipped tests that resulted in "unreachable code" errors. The other one is an error about a missing argument.

Implement basic webview support. This should allow us to iterate on the datascience controls using react. This commit creates a history pane that just looks like the default "react" page but works inside of a webview panel. Additionally it still makes it possible to load the index.html into a browser for debugging the react code standalone.

Implements the ability to talk to a jupyter kernel and execute cells.

DonJayamanne · 2018-10-26T20:54:54Z

        "pidusage": "^1.2.0",
+        "prismjs": "^1.15.0",
+        "promisify-node": "^0.5.0",
+        "react-json-tree": "^0.11.0",


Please move all of the npm dependencies that are required by the React components into devDependencies
Why?

None of the npm dependencies required by the react app need to be deployed with the extension.

E.g. if react-json-tree is only used in the react app, then there's no need to deploy this with the extension.

When you WebPack, it will pull all necessary dependencies and bundle into your app (so that solves dependencies for react. Else you'll have reacct-json-tree in both node_modules as well as your react bundle.

Okay will do

DonJayamanne · 2018-10-26T20:57:32Z

        "semver": "^5.5.0",
+        "styled-jsx": "^3.1.0",
        "sudo-prompt": "^8.2.0",
+        "temp": "^0.8.3",


Can't we use tmp instead of adding a new dependency?

Actually I can just use the IFileSystem.createTemporaryFile instead.

DonJayamanne · 2018-10-26T20:59:09Z

@@ -0,0 +1,52 @@
+class HelloWorldPlugin {


Please move these into build\datascience.

DonJayamanne · 2018-10-26T21:19:35Z

+// To learn more about the benefits of this model, read https://goo.gl/KwvDNy.
+// This link also includes instructions on opting out of this behavior.
+
+const isLocalhost = Boolean(


do we need this?
everything is local, so i dont see the need for service worker

Probably not anymore. The original intent was to be able to debug it using a server. But we haven't been doing that. I'll remove it.

able to debug it using a server

What do you mean?

DonJayamanne · 2018-10-26T21:30:00Z

+        "@nteract/transform-model-debug": "^3.2.3",
+        "@nteract/transform-plotly": "^3.2.3",
+        "@nteract/transforms": "^4.4.4",
+        "@types/prismjs": "^1.9.0",


@types/prismjs must go in devDependencies

Okay I'll move it.

DonJayamanne · 2018-10-26T21:30:37Z

    "dependencies": {
+        "@jupyterlab/services": "^3.1.4",
+        "@nteract/transform-dataresource": "^4.3.5",
+        "@nteract/transform-geojson": "^3.2.3",


Please see comment related to moving react dependencies into devDependencies (only if using in react alone).

Okay will do.

DonJayamanne · 2018-10-26T21:31:41Z

        "tmp": "^0.0.29",
        "tree-kill": "^1.2.0",
+        "typed-promisify": "^0.4.0",
+        "typed-react-markdown": "^0.1.0",


Please move type definitions into devDependencies.

DonJayamanne

I'm concerned about the test coverage.
We don't seem to have must tests for the new code

DonJayamanne · 2018-10-26T21:46:22Z

+
+// tslint:disable-next-line:no-function-expression
+(function() {
+    const origRequire = Module.prototype.require;


Note: This gets applied globally (i.e. this applies to all tests), hence might as well move into a common area.
Similar to what we do today in src/test/vscode-mock.ts and gets initialized in src/test/unittests.ts as follows:

export function runTests(testOptions?: { grep?: string; timeout?: number }) { vscodeMoscks.initialize(); `<CSS Mocks> initialize`

DonJayamanne · 2018-10-26T21:51:26Z

@@ -0,0 +1,165 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.


As these tests can take upto a minute, please move these into history.test.tsx.
Or move into history.functional.test.tsx
& then create a new test runner for these tests, we need to ensure the *.unit.test.ts tests still run fast without spawning processes, etc.
All *.unit.test.ts tests are considered OS agnostic.

These are OS agnostic but they don't require or use VS code. I guess I can create another test runner for now.

Yeah not sure how long that will take though. If we want to ship this by Wednesday I'm not sure if I can get this done and the other stuff I have to do by then.

No, lets discuss this on Monday.

These tests (when jupyter is present) take about 7 or 8 seconds.

Does this spawn any process or require python?
I believe they do, hence the reason they could take a while.

Yes they spawn jupyter using the python -m command.

DonJayamanne

Approved accidentally

…crosoft/vscode-python into dev/rchiodo/merge_datascience

brettcannon · 2018-10-29T17:07:19Z

What is history-react? Was all of that written from scratch or is this some external project that you are embedding? Is there any external project that is being embedded?

rchiodo · 2018-10-29T17:30:39Z

      What is history-react? Was all of that written from scratch or is this some external project that you are embedding? Is there any external project that is being embedded?

That's just a directory of the code we've written. It's not somebody else's code. It's our UI.

It's called history-react because we wanted it to be clear it was using reactjs.

rchiodo · 2018-10-29T17:32:44Z

There are new dependencies though. You can see them in package.json. We embed a whole bunch of react controls to get the UI to work.

brettcannon · 2018-10-29T18:08:31Z

@rchiodo "Embed" as in there are files you copied from somewhere else?

brettcannon

Marking as "request changes" to make sure we don't accidentally merge while we work out 3rd-party project details.

rchiodo · 2018-10-29T18:18:20Z

Well sort of. Most of the code is just npm installed. There is one set of regex's I copied from prismjs python code. (MIT licensed). However prism is also npm installed so I think it should be captured in the oss governance stuff.

rchiodo · 2018-10-29T21:14:29Z

Just to put it here in case we forget, the src\client\datascience\history-react\transforms.ts is essentially a copy of this file here:
https://github.com/nteract/nteract/blob/master/packages/transforms-full/src/index.js
but with the vegas stuff commented out.

brettcannon · 2018-10-29T22:49:01Z

@rchiodo should we have a news entry for #2302 in this PR?

rchiodo · 2018-10-29T22:51:56Z

That's probably a good idea. Now I have a number for the news entry I added. I'll renumber it.

IanMatthewHuff and others added 30 commits September 26, 2018 10:41

merge in changes from the old data science branch

640c2e6

Merge pull request #1 from vscode-python-datascience/dev/ianhu/DataSc…

b81aa44

…ienceChanges merge in changes from the old data science branch

Merge remote-tracking branch 'upstream/master'

7b7ed4a

Merge remote-tracking branch 'upstream/master' into dev/ianhu/upstrea…

0315a47

…mUpdate

Implement build system for react web pages

696198f

Tie this into vscode:prepublish Have normal build still build react parts Have normal hygiene still work on react parts

Skip defines for hygiene. They can't have a correct copyright

c5395ab

Merge pull request #4 from vscode-python-datascience/dev/ianhu/upstre…

e556da9

…amUpdate Upstream update and CI test

Merge remote-tracking branch 'origin/master' into dev/rchiodo/add_web…

bf9f4f8

…view_support

Merge pull request #5 from vscode-python-datascience/dev/rchiodo/add_…

a0106b3

…webview_support Adding building and hygiene support for react webview controls

Report Microsoft Python language server version to console during sta…

357adf7

…rtup (#2711) * Fixup naming inconsistencies * Add LanguageServerFolderService mock to other activationService tests.

Changed logic used to determine validity of debugger executable (#2708)

d154f77

For #2351

Added missing setting (for intellisense) (#2720)

2168762

Change blob containers for language server (#2721)

9c40658

Fix colon-triggered block formatting (#2724)

a735400

* Dispatch on-type formatters to prevent them from overriding each other * Move providers out of dispatcher, write tests * Add news entry

Provide a more thorough outline on how to get set up

a8acf9e

Adds support for code completion in the debug console window (#2716)

75725dc

Remove reporting of ls version in output panel (#2738)

a11419c

* Remove reporting of ls version * Remove file

Check for new versions on beta channel once a day (#2743)

3949853

For #2723

#1882: Use "conda activate" after 4.4.0. (#2717)

d7b95b6

We adjust to changes in 4.4.0.

Ensure relative paths to interpreters are prefixed with './' in setti…

12105e5

…ngs.json (#2750) * Prefix relative paths with './' * Ensure relative path is prefixed with ./

Clean up compiler errors. (#2760)

942e1c4

I'm seeing compiler errors that have slipped in over the last few weeks. This PR fixes them. All but one are skipped tests that resulted in "unreachable code" errors. The other one is an error about a missing argument.

Add jupyterserver communciation code (#8)

4becf22

Implements the ability to talk to a jupyter kernel and execute cells.

working code lens registration per file

6806485

hygiene cleanup

fec4998

working code lens command

3b11d4c

code lens cleanup

9687c6a

remove added file

3d60bf9

Merge branch 'master' into dev/ianhu/basicCodeLensSupport

97814e9

merge cleanup

279bd87

Fix hygiene issues

274fe81

DonJayamanne requested changes Oct 26, 2018

View reviewed changes

DonJayamanne approved these changes Oct 26, 2018

View reviewed changes

DonJayamanne requested changes Oct 26, 2018

View reviewed changes

Work on review comments

e1674e8

DonJayamanne requested changes Oct 26, 2018

View reviewed changes

Comment thread src/client/datascience/jupyterServer.ts Outdated

IanMatthewHuff and others added 5 commits October 26, 2018 16:26

feedback fixes from Don

ed1c7f7

remove dead code path

4a6f569

Fix some code review issues and get the history test to pass again

1795a66

Merge branch 'dev/rchiodo/merge_datascience' of https://github.com/Mi…

8064f24

…crosoft/vscode-python into dev/rchiodo/merge_datascience

Fix hygiene errors

054ac9c

Fix history unit test to run on linux

e2f091e

brettcannon removed request for MikhailArkhipov and jakebailey October 29, 2018 17:10

brettcannon suggested changes Oct 29, 2018

View reviewed changes

DonJayamanne approved these changes Oct 29, 2018

View reviewed changes

brettcannon approved these changes Oct 29, 2018

View reviewed changes

Remove copyright on nteract file

aebb6e9

Apply appropriate news entry number

f9ca6df

DonJayamanne merged commit e395064 into master Oct 30, 2018

brettcannon deleted the dev/rchiodo/merge_datascience branch October 31, 2018 17:20

lock Bot locked as resolved and limited conversation to collaborators Jul 30, 2019

		@@ -0,0 +1,165 @@
		// Copyright (c) Microsoft Corporation. All rights reserved.

Conversation

rchiodo commented Oct 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DonJayamanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DonJayamanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brettcannon commented Oct 29, 2018

Uh oh!

rchiodo commented Oct 29, 2018

Uh oh!

rchiodo commented Oct 29, 2018

Uh oh!

brettcannon commented Oct 29, 2018

Uh oh!

brettcannon left a comment

Choose a reason for hiding this comment

Uh oh!

rchiodo commented Oct 29, 2018

Uh oh!

rchiodo commented Oct 29, 2018

rchiodo commented Oct 26, 2018 •

edited

Loading