Skip to content
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

Add Vega 3 extension #15

Merged
merged 17 commits into from
Dec 6, 2017
Merged

Add Vega 3 extension #15

merged 17 commits into from
Dec 6, 2017

Conversation

hadim
Copy link
Contributor

@hadim hadim commented Jul 29, 2017

Still a WIP.

Please tell me if it's the correct repo for this package.

@hadim
Copy link
Contributor Author

hadim commented Jul 29, 2017

I can't get the extension to load in my JLab. This Javascript world is very new for me so forgive me if I miss something obvious.

Here is what I do:

$ cd jupyter-renderers/packages/vega3-extension/
$ npm install
$ npm run build 
$ jupyter labextension link .
$ jupyter lab build
$ jupyter lab  --no-browser --NotebookApp.token='' --dev-mode

To check whether the extension is loaded I look for the CSS class jp-RenderedVegaCommon3 in the HTML page but I can't find it. I only find the jp-RenderedVegaCommon (from the regular Vega 2 extension).

@hadim
Copy link
Contributor Author

hadim commented Jul 29, 2017

The extension is now loading correctly (the issue was coming from a typo in package.json, see jupyterlab/jupyterlab#2765 for a fix in the vega2 extension).

I can see the actions buttons but the plot does not show up.

screenshot from 2017-07-29 13-24-43

Here is the error displayed in the JS console when trying to load a Vegalite 2 plot.

logger.js:3 ERROR TypeError: Cannot read property 'datum' of undefined
    at layoutAxis (ViewLayout.js:147)
    at layoutGroup (ViewLayout.js:68)
    at ViewLayout.js:47
    at Array.forEach (<anonymous>)
    at ViewLayout.prototype.transform (ViewLayout.js:45)
    at ViewLayout.prototype.evaluate (Transform.js:52)
    at ViewLayout.prototype.run (Transform.js:33)
    at View.run (run.js:60)
    at View.prototype.run (View.js:96)
    at embed (embed.js:129)
log @ logger.js:3

No error is shown when loading Vega 3 plot (but the plot is not displayed).

@hadim
Copy link
Contributor Author

hadim commented Aug 1, 2017

@gnestor is this the correct repo for this extension?

@gnestor
Copy link
Collaborator

gnestor commented Aug 2, 2017

It is!! Thanks for this! I'll try it out now...

@gnestor
Copy link
Collaborator

gnestor commented Aug 4, 2017

Some notes:

  • You should be able to install and build the packages all from the project root using npm install and npm run build. This is important because lerna will hoist the dependencies and resolve version inconsistencies across extensions, etc.
  • I had to downgrade typescript to 2.3.1 in order to build json-extension. I ran into an obscure type error:
src/component.tsx(25,14): error TS2415: Class 'Component' incorrectly extends base class 'Component<IProps, IState>'.
  Types of property 'state' are incompatible.
    Type '{ filter: string; }' has no properties in common with type 'Readonly<IState>'.
display({
    'application/vnd.vega.v3+json': {
        "width": 400,
        "height": 200,
        "padding": {"top": 10, "left": 30, "bottom": 30, "right": 10},
        "data": [
            {
                "name": "table",
                "values": [
                    {"x": 1,  "y": 28}, {"x": 2,  "y": 55},
                    {"x": 3,  "y": 43}, {"x": 4,  "y": 91},
                    {"x": 5,  "y": 81}, {"x": 6,  "y": 53},
                    {"x": 7,  "y": 19}, {"x": 8,  "y": 87},
                    {"x": 9,  "y": 52}, {"x": 10, "y": 48},
                    {"x": 11, "y": 24}, {"x": 12, "y": 49},
                    {"x": 13, "y": 87}, {"x": 14, "y": 66},
                    {"x": 15, "y": 17}, {"x": 16, "y": 27},
                    {"x": 17, "y": 68}, {"x": 18, "y": 16},
                    {"x": 19, "y": 49}, {"x": 20, "y": 15}
                ]
            }
        ],
        "scales": [
            {
                "name": "x",
                "type": "ordinal",
                "range": "width",
                "domain": {"data": "table", "field": "x"}
            },
            {
                "name": "y",
                "type": "linear",
                "range": "height",
                "domain": {"data": "table", "field": "y"},
                "nice": True
            }
        ],
        "axes": [
            {"type": "x", "scale": "x"},
            {"type": "y", "scale": "y"}
        ],
        "marks": [
            {
                "type": "rect",
                "from": {"data": "table"},
                "properties": {
                    "enter": {
                        "x": {"scale": "x", "field": "x"},
                        "width": {"scale": "x", "band": True, "offset": -1},
                        "y": {"scale": "y", "field": "y"},
                        "y2": {"scale": "y", "value": 0}
                    },
                    "update": {
                        "fill": {"value": "orange"}
                    },
                    "hover": {
                        "fill": {"value": "yellow"}
                    }
                }
            }
        ]
    }
}, raw=True)
Uncaught (in promise) TypeError: Cannot read property '0' of undefined
  • If you check the "Allow edits from maintainers" checkbox in the right column, I will push some commits to this PR

@hadim
Copy link
Contributor Author

hadim commented Aug 4, 2017

Thank you for looking into this.

The "Allow edits from maintainers" was checked by default.

I can now build all the extensions in the same time but opening a file or into the notebook I still can't see the plot (only the 3 buttons at the bottom).

Feel free to push commits.

Also, an aside question: are the extensions from this repo going to be shipped by default with jupyterlab?

@hadim
Copy link
Contributor Author

hadim commented Aug 5, 2017

When dragging the mouse over the blank plot I got the following JS error:

CanvasHandler.js:183 Uncaught TypeError: Cannot read property 'marktype' of undefined
    at CanvasHandler.prototype.pick (CanvasHandler.js:183)
    at CanvasHandler.prototype.pickEvent (CanvasHandler.js:172)
    at CanvasHandler.mousemove (CanvasHandler.js:74)
    at HTMLCanvasElement.<anonymous> (CanvasHandler.js:24)

@gnestor
Copy link
Collaborator

gnestor commented Aug 10, 2017

I'm having an issue pushing commits to your branch:

grant:jupyter-renderers grant$ git push hadim vega3
ERROR: Authentication error: Authentication required: You must have push access to verify locks
error: failed to push some refs to 'https://github.com/hadim/jupyter-renderers.git'

Also, an aside question: are the extensions from this repo going to be shipped by default with jupyterlab?

Yes, that is the plan. Maybe not all of them, but definitely some. We still need to figure out the best way to do it (it will prob involve just adding the npm packages of each renderer to the jupyterlab package.json).

I am also seeing a blank space where the plot should be and the action buttons. I'm not seeing an errors (uncaught or caught), so I'm not sure what's wrong... 🤔

@hadim
Copy link
Contributor Author

hadim commented Aug 10, 2017

Do you have the maintainers permission? I have unchecked/checked the box. I'll add you to my fork if does not work.

@gnestor
Copy link
Collaborator

gnestor commented Aug 10, 2017

Maybe I don't....

@ellisonbg Can you check to see if I have maintainer permissions for this repo?

@ellisonbg
Copy link

ellisonbg commented Aug 11, 2017 via email

@gnestor
Copy link
Collaborator

gnestor commented Aug 11, 2017

Still not working... @ellisonbg Can you make sure I have push access?

@hadim
Copy link
Contributor Author

hadim commented Aug 11, 2017

I don't understand why it doesn't work. I have added you to my repo @gnestor .

@gnestor
Copy link
Collaborator

gnestor commented Aug 11, 2017

Ok, that worked! Thanks @hadim!

@gnestor
Copy link
Collaborator

gnestor commented Aug 11, 2017

So the plots are still not rendering but at least we can work off of the same source. I'm not sure how to debug this since there are no exceptions...

@domoritz We are trying to create a new renderer extension for Vega 3 and Vega-lite 2 using vega-embed 3. When we render a valid Vega 3 spec, we can see the action buttons but no plots (just an empty space where the plot should be). Do you think this could be the result of having the vega2-extension also loaded? I have tried removing vega2-extension from JupyterLab and I get the same result AFAIK...

@ellisonbg
Copy link

@hadim that for contributing this - glad to see it moving forward1

@domoritz
Copy link
Member

@gnestor I'm surprised that you don't get any error messages if no chart appears. Have you tried building a simple html test page outside of jupyter lab to see whether the chart renders correctly there?

@hadim
Copy link
Contributor Author

hadim commented Aug 11, 2017

Also consider my very low skills in JS. I have only copy/pasted the Vega 2 extension and removed what I considered being useless. So a careful review might be required by you guys. I am on vacation at the moment and won't be able to hack this PR before 10 days or so.

"@jupyterlab/rendermime-interfaces": "^0.3.0",
"@phosphor/coreutils": "^1.2.0",
"@phosphor/widgets": "^1.3.0",
"vega-embed": "^3.0.0-beta.19"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to use the promise API, you have to update to embed beta 20.

@hadim
Copy link
Contributor Author

hadim commented Oct 18, 2017

I apologize for not being able to work on this at the moment guys.

I am glad someone is taking the lead.

@domoritz
Copy link
Member

domoritz commented Oct 18, 2017

@hadim @gnestor I have a pr for Vega-Embed to be converted to typescript (vega/vega-embed#49). I hope that will make things easier over here. Let me know if I can help in any other way.

@gnestor
Copy link
Collaborator

gnestor commented Oct 19, 2017

@domoritz That will super helpful! Look forward to it.

Update: There was an issue with jupyterlab loading 2 different versions of vega-embed, but that's resolved with jupyterlab/jupyterlab#3126. It looks like I'm back to the vega3 rendering blank charts issue from August (#15 (comment)). Again, no exceptions, the same spec renders fine in Vega Editor. @domoritz I could definitely use your help but I haven't been able to reproduce it outside of jupyterlab, so it would be great if you could checkout https://github.com/blink1073/jupyterlab/tree/fix-watch-mode of jupyterlab and https://github.com/hadim/jupyter-renderers/tree/vega3 of jupyterlab to reproduce.

@gnestor
Copy link
Collaborator

gnestor commented Oct 20, 2017

@domoritz jupyterlab/jupyterlab#3126 has been merged, so you can test against jupyterlab master 👍

@blink1073
Copy link
Contributor

It is in the v0.28.5 release.

@@ -1,7 +1,7 @@
{
"compilerOptions": {
"declaration": true,
"noImplicitAny": true,
"noImplicitAny": false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessary since we added the typings below.

@blink1073
Copy link
Contributor

💥, this works like a charm using JupyterLab master right now, cutting a 0.30 rc of JupyterLab now.

@domoritz
Copy link
Member

domoritz commented Dec 4, 2017

Sweet. I'm looking forward to having a first version working in Jupyter Lab.

@gnestor
Copy link
Collaborator

gnestor commented Dec 4, 2017

Excellent!! I will test against master/v0.30.0 right now. So happy about this!

@ellisonbg
Copy link

ellisonbg commented Dec 4, 2017 via email

@gnestor
Copy link
Collaborator

gnestor commented Dec 4, 2017

It's working! Just rebased so this should be ready to merge.

@blink1073 @ellisonbg Do we want to move vega3-extension to jupyterlab repo?

@blink1073
Copy link
Contributor

I'll defer to Brian on this one.

@gnestor
Copy link
Collaborator

gnestor commented Dec 6, 2017

I'm going to merge and we can decide whether to move to jupyterlab repo after...

@gnestor gnestor merged commit 23f6c48 into jupyterlab:master Dec 6, 2017
@gnestor gnestor deleted the vega3 branch December 6, 2017 20:18
@gnestor
Copy link
Collaborator

gnestor commented Dec 6, 2017

Thanks @hadim @domoritz and @blink1073!

@blink1073
Copy link
Contributor

Beverages of choice for all!

@domoritz
Copy link
Member

domoritz commented Dec 6, 2017

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants