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 CONTRIBUTING.md #161

Merged
merged 7 commits into from Jun 12, 2019
Merged

Conversation

jtpio
Copy link
Member

@jtpio jtpio commented May 27, 2019

Fixes #134.

This is the set of commands I've used so far and they have been enough for regular development.

@choldgraf @SylvainCorlay @maartenbreddels feel free to jump in and add anything missing.
I'll do a second pass on it a bit later today / this week.

@choldgraf
Copy link
Contributor

I'll take another pass at getting this set up for myself this week. I still haven't been able to get Voila working in dev mode :-/

@jtpio jtpio changed the title [WIP] Add CONTRIBUTING.md Add CONTRIBUTING.md May 28, 2019
@jtpio
Copy link
Member Author

jtpio commented May 28, 2019

Second pass done. This should be ready to go.

@choldgraf did you give it another try?

@choldgraf
Copy link
Contributor

will give this a shot today

@choldgraf
Copy link
Contributor

Some notes:

  • Install instructions worked this time w/ pip install -e . dunno if that was a change on my end or your end but 👍
  • I got an error about not having the right version of pygments installed when I tried to run voila. running pip install -U Pygments got it to work. Perhaps that should be added in here?

@jtpio
Copy link
Member Author

jtpio commented May 30, 2019

* I got an error about not having the right version of pygments installed when I tried to run `voila`. running `pip install -U Pygments` got it to work. Perhaps that should be added in here?

It should normally be installed automatically (via jupyterlab_pygments)

@choldgraf
Copy link
Contributor

Yeah - I think I just hadn't updated it recently which is why I had an outdated version. Maybe just add a "conda update XXX" for the required conda packages? Though I agree most developer types could have figured it out

@choldgraf
Copy link
Contributor

Ah one more point here after experimenting a bit more. It doesn't seem like this installs the voila server extension (at least, I'm getting a 500 error when I run jupyter lab and hit /voila. Is that right?

@SylvainCorlay
Copy link
Member

I got an error about not having the right version of pygments installed when I tried to run voila. running pip install -U Pygments got it to work. Perhaps that should be added in here?

Both the conda recipe and setup.py for jupyterlab_pygments require pygments>=2.4.1 so it is weird that you could get it with an older version. I don't think we should specify anything though.

@SylvainCorlay
Copy link
Member

Ah one more point here after experimenting a bit more. It doesn't seem like this installs the voila server extension (at least, I'm getting a 500 error when I run jupyter lab and hit /voila. Is that right?

Indeed, development installations don't install data files. Jupyter server extensions rely on a configuration directory system in share, where the assets for the voila jupyterlab extension are placed.

In the case of a development installation, the extension can be installed and enabled with:

  • for the classic notebook server
jupyter serverextension install --py --sys-prefix --symlink voila
jupyter serverextension enable --py --sys-prefix voila

the --symlink is not supported on windows.

  • for jupyter_server
jupyter extension install --py --sys-prefix --symlink voila
jupyter extension enable --py --sys-prefix voila

@maartenbreddels
Copy link
Member

I think we can fix all that by adding a custom devel command in setup.py, and symlinks on windows 10 seem to work fine, but requires admin mode (I think). Maybe a 'good first issue' to open?

@SylvainCorlay
Copy link
Member

I think we can fix all that by adding a custom devel command in setup.py, and symlinks on windows 10 seem to work fine, but requires admin mode (I think). Maybe a 'good first issue' to open?

Yes, that would make sense.

In the mean time, we could add these instructions for those who want to use the server extensions in development mode.

@jtpio
Copy link
Member Author

jtpio commented Jun 1, 2019

jupyter serverextension install --py --sys-prefix --symlink voila

Isn't the jupyter serverextension enable command enough? (no install subcommand for server extensions)

We can also add instructions for the notebook extension (which adds the "Voila" button to the classic notebook).
And when #40 is merged instructions for the JupyterLab extension.

@yuvipanda
Copy link

If you're using this with jupyter_server (instead of notebook), you'll need to run jupyter extension enable instead of jupyter serverextension enable

@jtpio
Copy link
Member Author

jtpio commented Jun 5, 2019

Updated based on #182 and @yuvipanda's comment.

CONTRIBUTING.md Outdated
For Jupyter Server:

```bash
jupyter extension enable voila --sys-prefix [--py | --user]
Copy link
Member

Choose a reason for hiding this comment

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

remove --user.

Copy link
Member

Choose a reason for hiding this comment

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

It seems to me that --py is always required?

Copy link
Member Author

Choose a reason for hiding this comment

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

--py throws the following for me:

KeyError: 'The Python module voila does not include any valid server extensions'

Copy link
Member Author

Choose a reason for hiding this comment

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

Full command: jupyter serverextension enable voila --sys-prefix --py

Copy link
Member

Choose a reason for hiding this comment

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

indeed we don't have an entry point for a jupyter server extension in __init__.py. We should probably just remove the --py.

@SylvainCorlay SylvainCorlay merged commit 90f7e4d into voila-dashboards:master Jun 12, 2019
SylvainCorlay pushed a commit that referenced this pull request Jun 12, 2019
* Add CONTRIBUTING.md (still wip)

* Add cd js/ to CONTRIBUTING.md

* Update CONTRIBUTING.md

* Add section about extensions

* Update server extension instructions

* Add "Tests" section

* Remove --user, add JupyterLab extension
@choldgraf
Copy link
Contributor

Woot, thanks for this @jtpio

@jtpio jtpio deleted the contributing branch June 13, 2019 07:02
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.

Improve development installation
5 participants