/matplotlib

Documentated dependencies to the doc and remove unecessary dependencies. #7049

Merged
merged 4 commits into from Sep 8, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
  v2.0.0
7 participants
@NelleV @matthew-brett @jenshnielsen @tacaswell @QuLogic @mdboom @afvincent
@NelleV
Contributor

NelleV commented Sep 6, 2016 • edited

Following discussion in #7040, I have documented the dependencies for building the documentation and removed the basemap example.

  • Add the doc-pip-requirement.txt file and document it.
  • Have travis refer to the doc-requirement file.
  • Add a check that all the dependencies are installed before running the documentation build to avoid crashes in the middle of the build.
  • Document the --allowsphinxwarnings

mdboom added the needs_review label Sep 6, 2016

afvincent added the Documentation label Sep 6, 2016

@matthew-brett
Contributor

matthew-brett commented Sep 6, 2016

Did you mean to add a doc-requirements.txt file as well?
@NelleV
Contributor

NelleV commented Sep 6, 2016

@matthew-brett I don't think it'd hurt, if some people use this kind of tools.
@matthew-brett
Contributor

matthew-brett commented Sep 6, 2016

The pip requirements file format is obvious enough that you could refer to that for the dependency list - as in "see the doc-requirements.txt file for the dependencies to install". That way you don't have to keep two versions of the dependencies, one in the requirements file and another in the docs.

Tom also suggested adapting the .travis.yml file to install the doc dependencies from the doc-requirements.txt file, which will also make sure it's up to date and working.
@NelleV
Contributor

NelleV commented Sep 6, 2016

Sounds like a plan.
@matthew-brett
Contributor

matthew-brett commented Sep 6, 2016

Nelle - I did a PR to your branch with the requirements file.

@QuLogic QuLogic commented on an outdated diff Sep 6, 2016

View full outdated diff
doc/conf.py
@@ -52,6 +52,17 @@
raise ImportError("No module named numpydoc - you need to install "
"numpydoc to build the documentation.")
+try:
+ import colorspacious
+except ImportError:
+ raise ImportError("No module named colorspacious - you need to install "
+ "colorspacious to build the documentation")
+
+try:
+ import mock
@QuLogic

QuLogic Sep 6, 2016

Member

mock is only required on Python 2.7; maybe this test should be merged with the import on line 291?
@NelleV
Contributor

NelleV commented Sep 6, 2016

@matthew-brett isn't pillow missing from your doc-requirements.txt file?

tacaswell added this to the 2.0 (style change major release) milestone Sep 7, 2016

@matthew-brett matthew-brett and 1 other commented on an outdated diff Sep 7, 2016

View full outdated diff
doc/README.txt
@@ -1,11 +1,45 @@
maptlotlib documentation
========================
+
+Building the documentation
+--------------------------
+
+A list of dependencies can be found in ../doc-requirements.txt.
+
+All of these dependencies can be installed through pip::
+
+ pip install sphinx numpydoc ipython mock colorspacious
@matthew-brett

matthew-brett Sep 7, 2016

Contributor

This could be pip install -r ../doc-requirements.txt.

@NelleV

NelleV Sep 7, 2016

Contributor

Yeah… I'd just really really really don't like giving (or running) commands that are not explicit… But I'll update this to use the doc-requirements.txt

@NelleV

NelleV Sep 7, 2016

Contributor

I am just waiting to check travis is happy.

@matthew-brett matthew-brett and 1 other commented on an outdated diff Sep 7, 2016

View full outdated diff
doc/conf.py
@@ -52,6 +52,19 @@
raise ImportError("No module named numpydoc - you need to install "
"numpydoc to build the documentation.")
+try:
+ import colorspacious
+except ImportError:
+ raise ImportError("No module named colorspacious - you need to install "
+ "colorspacious to build the documentation")
+try:
+ from unittest.mock import MagicMock
+except:
@matthew-brett

matthew-brett Sep 7, 2016

Contributor

except ImportError:?

@NelleV

NelleV Sep 7, 2016

Contributor

I didn't write this code, but I would indeed assume so… Thanks for the catch!

NelleV and others added some commits Sep 6, 2016

@NelleV NelleV DOC: documentated and simplified doc build 
- Removed basemap example, as it contains a basemap dependency. Basemap being
  currently quite hard to install, it adds unecessary burden for contributors
  to the documentation.
- Documented the documentation build dependencies.
- Checks that dependencies are installed *before* running the build.
- Documented build options.

fix #7040
 ede1959
@matthew-brett @NelleV matthew-brett MAINT: add doc-requirements file 
Add pip requirements file for documentation dependencies.

[skip ci]
 363ce54
@NelleV NelleV DOC documents the requirements.txt & updates travis 
- This PR documents the doc-requirements file and updates travis to use this
  file during the installation process.
- Added pillow requirements.
- Cleaned up conf.py a bit.

Some checks were not successful

1 failing and 2 successful checks
continuous-integration/appveyor/pr — AppVeyor build failed
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage remained the same at 70.362%
Details
faec918

NelleV changed the title from [WIP] Documentated dependencies to the doc and remove unecessary dependencies. to [MRG] Documentated dependencies to the doc and remove unecessary dependencies. Sep 7, 2016

@jenshnielsen jenshnielsen and 1 other commented on an outdated diff Sep 7, 2016

View full outdated diff
doc/README.txt
+
+or conda::
+
+ conda install sphinx numpydoc ipython mock colorspacious pillow
+
+To build the HTML documentation, type ``python make.py html`` in this
+directory. The top file of the results will be ./build/html/index.html
+
+**Note that Sphinx uses the installed version of the package to build the
+documentation**: matplotlib must be installed *before* the docs can be
+generated. Even if that is the case, one of the files needed to do this,
+'../lib/matplotlib/mpl-data/matplotlibrc', is not version controlled, but
+created when matplotlib is built. This means that the documentation cannot be
+generated immediately after checking out the source code, even if matplotlib
+is installed on your system: you will have to run ``python setup.py build``
+first.
@jenshnielsen

jenshnielsen Sep 7, 2016

Owner

As far as I know this should no longer be true following the merge of #6530. Do you still need this?

@NelleV

NelleV Sep 7, 2016

Contributor

As far as I know, when @keszybz tried to compile the matplotlib doc yesterday, we ran into this problem.

@NelleV

NelleV Sep 7, 2016

Contributor

I'll test.

@NelleV

NelleV Sep 7, 2016

Contributor

Somehow I am unable to tests… Whether I build the documentation or not, the build stalls after some time. I have no clue why or how to identify what the problem is.

@jenshnielsen

jenshnielsen Sep 7, 2016

Owner

The build of matplotlib? How do you build and install matplotlib. pip install .? setup.py install?

@NelleV

NelleV Sep 7, 2016

Contributor

python setup.py install. And I had no problem yesterday. I tried on a fresh checkout, fresh installed.
Maybe my computer got tired of building matplotlib's documentation…

@NelleV

NelleV Sep 7, 2016

Contributor

The problem is that some examples try to fetch data on internet and the website is down.
I don't think we should be have download from internet required to build the documentation.
I personally am in favor of removing all the examples that rely on downloading data on internet.

It turns out that all of these examples are from the finance module, which is deprecated.

(Thanks so much to @rougier for the help in debugging).

@jenshnielsen

jenshnielsen Sep 7, 2016

Owner

Not building the finance examples seems fine to me. In the medium term I think we should split the finance module out into it's own package where it can be maintained (or not) by anyone with interest in that.

For now I suggest moving the finance examples to their own folder. Unless that folder is added to mpl_example_sections in https://github.com/matplotlib/matplotlib/blob/master/doc/conf.py they won't be build as part of the docs build.

Alternatively we can also just add a # -*- noplot -*- clause to the first line of the relevant examples.

@NelleV

NelleV Sep 7, 2016

Contributor

@rougier created a PR to remove those examples: #7057
This module being deprecated I think it is safe to remove the plots, considering they are in our history.
I don't think keeping files that aren't used is a good idea. We then end up with portions of code from PhD thesis that have never run and yet survive for dozens of years in matplotlib before anyone noticed.

@jenshnielsen

jenshnielsen Sep 7, 2016

Owner

We still get lots of requests from users that use the finance module. Just deleting it would be very user hostile

@NelleV

NelleV Sep 7, 2016

Contributor

We are not deleting the finance module. We are deleting the example that rely on downloading data from internet.

@NelleV

NelleV Sep 7, 2016

Contributor

Let's move this discussion to #7057 :)

rougier referenced this pull request Sep 7, 2016

Merged

FIX: Removed financial demos that stalled because of yahoo requests #7057

@NelleV NelleV FIX no need to build mpl before building the doc

Some checks were not successful

1 failing and 2 successful checks
continuous-integration/appveyor/pr — AppVeyor build failed
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage decreased (-0.002%) to 70.36%
Details
633a2e4
@NelleV
Contributor

NelleV commented Sep 7, 2016

@jenshnielsen Confirming there is no need to build mpl anymore.
Removed this part of the documentation.
@jenshnielsen
Owner

jenshnielsen commented Sep 7, 2016

Great 👍 on this. It's a much needed improvement
@NelleV
Contributor

NelleV commented Sep 8, 2016

appveyor's failure seems unrelated.

@tacaswell tacaswell merged commit 2d1e51f into matplotlib:master Sep 8, 2016
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.002%) to 70.36%
Details

tacaswell removed the needs_review label Sep 8, 2016

@tacaswell
Owner

tacaswell commented Sep 8, 2016

This does not backport cleanly to 2.x due to 2.x using the travis wheelhouse and installing nose from my fork (done a long time ago to fix a 3.6 compatibility issue). I do not have time to sort this out right now, will make an issue.

tacaswell referenced this pull request Sep 8, 2016

Closed

backport #7049 #7065

NelleV deleted the unknown repository branch Sep 8, 2016

@tacaswell tacaswell added a commit that referenced this pull request Sep 12, 2016

@tacaswell tacaswell Merge pull request #7049 from NelleV/doc_readme 
MNT: Documented dependencies to the doc and remove unnecessary dependencies
Conflicts:
	.travis.yml
	  backported master version of conflicts
 05f8434
@tacaswell
Owner

tacaswell commented Sep 12, 2016

backported to v2.x as 05f8434

QuLogic changed the title from [MRG] Documentated dependencies to the doc and remove unecessary dependencies. to Documentated dependencies to the doc and remove unecessary dependencies. Dec 7, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment