-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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
Conversation
Did you mean to add a |
@matthew-brett I don't think it'd hurt, if some people use this kind of tools. |
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 |
Sounds like a plan. |
Nelle - I did a PR to your branch with the requirements file. |
"colorspacious to build the documentation") | ||
|
||
try: | ||
import mock |
There was a problem hiding this comment.
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?
@matthew-brett isn't pillow missing from your doc-requirements.txt file? |
|
||
All of these dependencies can be installed through pip:: | ||
|
||
pip install sphinx numpydoc ipython mock colorspacious |
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
- 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.
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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll test.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)
@jenshnielsen Confirming there is no need to build mpl anymore. |
Great 👍 on this. It's a much needed improvement |
appveyor's failure seems unrelated. |
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. |
MNT: Documented dependencies to the doc and remove unnecessary dependencies Conflicts: .travis.yml backported master version of conflicts
backported to v2.x as 05f8434 |
Following discussion in #7040, I have documented the dependencies for building the documentation and removed the basemap example.