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

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

Merged
merged 4 commits into from
Sep 8, 2016
Merged

Conversation

NelleV
Copy link
Member

@NelleV NelleV commented Sep 6, 2016

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

@matthew-brett
Copy link
Contributor

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

@NelleV
Copy link
Member Author

NelleV commented Sep 6, 2016

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

@matthew-brett
Copy link
Contributor

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
Copy link
Member Author

NelleV commented Sep 6, 2016

Sounds like a plan.

@matthew-brett
Copy link
Contributor

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

"colorspacious to build the documentation")

try:
import mock
Copy link
Member

Choose a reason for hiding this comment

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

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

@NelleV
Copy link
Member Author

NelleV commented Sep 6, 2016

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

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

All of these dependencies can be installed through pip::

pip install sphinx numpydoc ipython mock colorspacious
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I am just waiting to check travis is happy.

NelleV and others added 3 commits September 7, 2016 01:23
- 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
Add pip requirements file for documentation dependencies.

[skip ci]
- 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.
@NelleV NelleV changed the title [WIP] Documentated dependencies to the doc and remove unecessary dependencies. [MRG] Documentated dependencies to the doc and remove unecessary dependencies. Sep 7, 2016
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.
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll test.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

@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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's move this discussion to #7057 :)

@NelleV
Copy link
Member Author

NelleV commented Sep 7, 2016

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

@jenshnielsen
Copy link
Member

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

@NelleV
Copy link
Member Author

NelleV commented Sep 8, 2016

appveyor's failure seems unrelated.

@tacaswell tacaswell merged commit 2d1e51f into matplotlib:master Sep 8, 2016
@tacaswell
Copy link
Member

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 tacaswell mentioned this pull request Sep 8, 2016
@NelleV NelleV deleted the doc_readme branch September 8, 2016 15:59
tacaswell added a commit that referenced this pull request Sep 12, 2016
MNT: Documented dependencies to the doc and remove unnecessary dependencies
Conflicts:
	.travis.yml
	  backported master version of conflicts
@tacaswell
Copy link
Member

backported to v2.x as 05f8434

@QuLogic QuLogic changed the title [MRG] Documentated dependencies to the doc and remove unecessary dependencies. 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants