Make sure notebook is pip installed when building docs

[mriedem]

The instructions on building the docs fail to mention that
the notebook package itself needs to be installed.

This updates the instructions to pip install the local notebook
package include the docs dependencies using the docs extra
install package list.

Closes #5741

[mriedem]

@kevin-bates this fixes the pip-based instructions but doesn't resolve the issue for conda, which I don't have setup locally to test. However, looking at the conda install docs it looks like this might be the fix for that?

diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index 5d9a2ecd3..3ee8a49fa 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -176,6 +176,7 @@ containing all the necessary packages (except pandoc), use::
 
     conda env create -f docs/environment.yml
     conda activate notebook_docs  # Linux and OS X
+    conda install -n notebook_docs .
     activate notebook_docs         # Windows
 
 .. _conda environment:

I'm not sure if -n notebook_docs is necessary if you've already activated the notebook_docs environment (it wouldn't be with something like an activated virtualenv which I'm used to), so it's just a guess. Feel free to update this PR if you can verify the conda instruction fix.

[kevin-bates]

Thanks Matt - I'm not very familiar with conda (and pip for that matter) myself. In looking into the equivalent of pip install . in conda I found there isn't really any other than to be sure to conda install pip first, then you can issue pip install ..

Since pip is installed by virtue of creating the environment with the specified file, I think the instructions for conda could be listed as follows...

     conda env create -f docs/environment.yml
     conda activate notebook_docs  # Linux and OS X
     activate notebook_docs        # Windows
     pip install .

At first, I could not reproduce the issue relative to conda but that was because I had notebook installed in my base conda env - which acts as a fallback if my understanding is correct. Once I removed notebook from my base env, then I could reproduce the issue and confirmed that pip install . allowed for the docs build to proceed.

If you don't mind adding this to the PR, that would be great. I'll probably ask for an additional reviewer relative to the conda/pip stuff to be sure no additional notes are necessary or anything like that.

I'm not sure if -n notebook_docs is necessary if you've already activated the notebook_docs environment

Once activated, all actions are performed against the activated env, no need to name it explicitly.

[mriedem]

If you don't mind adding this to the PR, that would be great.

Done, thanks for investigating.

[kevin-bates]

These changes look good and test out for me - thank you!

@blink1073 - I'm asking for your review with the hope you can sanity-check my comment to ensure we don't need additional text regarding the use of pip within a conda env. Thank you.

[Zsailer]

With the new extras_require arg in the setup.py, you should be able to just call:

pip install .[docs]

This will install notebook, its dependencies, and the docs dependencies.

It's a bit cleaner than installing from the requirements.txt files. We might consider dropping these files altogether.

[Zsailer]

It's a bit cleaner than installing from the requirements.txt files. We might consider dropping these files altogether.

We might consider doing the same for docs/environment.yml.

Instead, we could suggest the following chunk:

conda create -n notebook_docs pip
conda activate notebook_docs         # Linux and OS X
activate notebook_docs               # Windows
pip install .[docs]

[mriedem]

With the new extras_require arg in the setup.py, you should be able to just call:

Cool, I didn't realize that existed. That would make the pip-based setup instructions for building docs simpler.

One question, I noticed that doc-requirements.txt includes sphinx-rtd-theme but the docs extra in setup.py doesn't. Is it missing from setup.py or irrelevant in doc-requirements.txt? I'm assuming maybe the former since it's also listed in the conda docs/environment.yml.

Let me know if you'd like me to change this PR to use pip install .[docs] and remove docs/requirements.txt. Note that travis config would also need to be updated:

https://github.com/jupyter/notebook/blob/master/.travis.yml#L35

[mriedem]

Let me know if you'd like me to change this PR to use pip install .[docs] and remove docs/requirements.txt.

Alternatively, we could go with the fix here and follow up with a larger PR to remove docs/environment.yml and docs/doc-requirements.txt.

[Zsailer]

We'll need to add the sphinx-rtd-theme dependency in setup.py. Would you mind adding that in this PR?

Then, let's update the docs to use the pip install .[docs] command here in this PR.

We'll save removing the requirements files for a separate PR.

[kevin-bates]

Thank you @Zsailer - I agree that's much cleaner and essentially alleviates the need for Steve's review, so I'm going to remove that request. Thanks for your help!

[kevin-bates]

Hi Matt - sorry about the delay here. Would you mind resolving the conflict in setup.py, then we'll get this merged? Thanks.

[mriedem]

Hi Matt - sorry about the delay here. Would you mind resolving the conflict in setup.py, then we'll get this merged? Thanks.

Done, thanks for the heads up.

[kevin-bates]

LGTM - thank you.