Beautified frontpage plots and two pylab examples #7740

Merged
merged 1 commit into from Jan 13, 2017

Conversation

Projects
None yet
9 participants
Contributor

vollbier commented Jan 4, 2017

This addresses bug #7704: The reason for the thick lines in the frontpage plots seem to have to do with figure size. I left the default figsize ([6.4, 4.8]) and changed the dpi parameter in savefig() from 100 to 25 dpi (which is results roughly in the same number of pixels in the image).

I move the generated images by hand to the doc/_static directory. Should this happen automatically somehow?

I also tried to make the polar_demo.py and pie_demo_features.pylook nicer.
It does not seem possible to pass a list of strings to autopct parameter of pie(). Thus the hack to change the text properties afterwards. I'm not sure, if that's suited for a showcase. I also could delete it, as @efiring suggested. Furthermore, I'm not completely sure, if the radial positioning of the labels in pie() is always correct.

I did not succeed at changing the text appearance in table_demo.py.

Owner

tacaswell commented Jan 4, 2017

Can you either match the pixel size exactly or change the values in _static/mpl.css div.responsive_screenshots to match these sizes?

It would be better if it automatically copied and regenerated these on every build.

codecov-io commented Jan 5, 2017 edited

Current coverage is 62.12% (diff: 100%)

Merging #7740 into master will not change coverage

@@             master      #7740   diff @@
==========================================
  Files           174        174          
  Lines         56028      56028          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits          34805      34805          
  Misses        21223      21223          
  Partials          0          0          

Powered by Codecov. Last update 1fa4dd7...cc66626

Owner

tacaswell commented Jan 5, 2017

@vollbier Could you squash this into a single commit (to avoid having the extra binary images in the history)? I can also do it if you like.

@vollbier vollbier Fixed frontpage plots and generate them together with the docs (bug #…
…7704).

* Also fixed two pylab examples.
* Fixed _static/mpl.css div.responsive_screenshots
* Generate frontpage pngs automatically on html build.
* Also made the file doc/make.py more PEP8 conform und made the linter happier. That consisted mainly of adding blank lines and docstrings to the functions.
cc66626
Contributor

vollbier commented Jan 6, 2017

@tacaswell No problemo.

Member

phobson commented Jan 11, 2017

I approved this as well, but would like another set of eyes on it. I was able to build the docs and things indeed look better, but this touches enough sphinx code that I hesitate to merge it myself.

Contributor

NelleV commented Jan 11, 2017

I really recommand against change the make.py. I think the goal is to move to the "modern" way for sphinx to build, and in additition of making our current documentation build system hard to maintain and not standard, this will break when we finalize the migration to make.

Contributor

NelleV commented Jan 11, 2017

Understand my last comment as: I am strongly against part of this patch unless it builds upon our current gallery system and avoids modifying the make.py.

Owner

tacaswell commented Jan 11, 2017

I am not super concerned about the changes to make.py.

make.py is a big chunk of technical debt no matter what way you look at it. It we want to auto-generate the main page images (which are very different than a gallery example) we will have to have a custom build target or write a sphinx extension to do it. When we move to the modern makefile we can decide to keep this complexity (because auto-generating the main page images is a pretty reasonable thing to do) or drop it and go back to static images. In either case, there is little future cost to this now.

Contributor

vollbier commented Jan 12, 2017

I don't have any strong opinion about modifying make.py or not. I took a look at the current doc building structure: It seems like a big chunk of work to get it "sphinx-conform" - it's probably better to do that in a separate pull request.

Contributor

NelleV commented Jan 12, 2017

@tacaswell I disagree with you. Most scientific projects manage just fine with normal sphinx extensions and sphinx.

Is there an interest in moving to the up-to-date build system?

Contributor

dopplershift commented Jan 12, 2017

I think the discussion of what's needed for matplotlib's docs going forward is easily derailed in this forum, as well as off-topic for this PR. I vote we punt that to one of the weekly dev hangouts (open to anyone interested).

As far as this PR is concerned, I agree make.py is a mess, so let's just get the changes in it necessary to get the front page how we want for 2.0.

Contributor

NelleV commented Jan 12, 2017

There is absolutely no need to change the make.py for the sake of this PR. The images can be generated using the gallery system, and linked back properly to the front page if necessary. The reason I did not do this in my original PR updating those plots is that this will add 4 more plots to the gallery, which already has hundreds of example and it seemed like not such a good idea.

tacaswell changed the title from Beautified frontpage plots and two pylab examples to [MRG+2] Beautified frontpage plots and two pylab examples Jan 13, 2017

Owner

tacaswell commented Jan 13, 2017

I suggested in my second comment that it would be better to automatically generate these files (which @vollbier did in a very direct way). As I said above, I do not think this change moves the needle on the level of technical debt we have in make.py and is an improvement to building the docs.

make.py is a bit of an anachronism, but it works reliably. Updating it is probably a good idea, but not near the top of my internal list of priorities.

I am strongly in favor of merging this as-is, but given @NelleV 's concerns would at least one more senior dev to weigh in.

Member

WeatherGod commented Jan 13, 2017

Contributor

NelleV commented Jan 13, 2017

Changing the make.py is a terrible idea because it is a file that inherently does not belong to matplotlib. The modifications embedded in it has made my life hell in moving forward to the new makefile system, and it is because we keep doing these hacks that the project is so hard to maintain.

There is a way to generate the images without modifying the make.py. I am suggesting using the tool that has been build for this purpose to do this instead of modifying the make.py.

efiring changed the title from [MRG+2] Beautified frontpage plots and two pylab examples to Beautified frontpage plots and two pylab examples Jan 13, 2017

@efiring efiring merged commit a02e303 into matplotlib:master Jan 13, 2017

5 checks passed

codecov/patch Coverage not affected when comparing 1fa4dd7...cc66626
Details
codecov/project 62.12% (+0.00%) compared to 1fa4dd7
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 62.121%
Details
Contributor

NelleV commented Jan 13, 2017

I really would have appreciated not merging this PR until we reached a consensus.
This breaks my patch for no good reason, and I am not putting more effort into the migration of the makefile.

Owner

efiring commented Jan 13, 2017

The real problem with this is not the change to make.py, which is irrelevant, it is that the pie chart demo is still an embarrassment, though not as bad as it used to be.
Here is v2.x:
figure_2_v2 x
And below is master after the merge:
figure_2_master
Not only is it still "chart junk", it is still incompetent chart junk. The incompetence is in the Matplotlib implementation, not in the writing of the example script. Can we please simply delete this example? Would we want anyone to make a plot like this and advertise it as "made using Matplotlib"?

Owner

tacaswell commented Jan 13, 2017

👍 to removing the pie chart demo.

Owner

efiring commented Jan 13, 2017

Contributor

NelleV commented Jan 13, 2017

The migration to the Makefile. I closed the pull request, as the migration to Makefile depends on the migration of sphinx-gallery. The branch is still on my fork if someone wants to pick up the work, but considering the amount of work it requires, I doubt you'll find volunteers.

Member

phobson commented Jan 13, 2017

@NelleV is that #6663?

Contributor

NelleV commented Jan 13, 2017

Yes. I think the last remaining point is the sphinx-gallery migration. There are only around 350 examples to tackle there.

Owner

efiring commented Jan 13, 2017

Maybe the approach we need to take here is to think of the migration as gradually creating a sphinx gallery in parallel, adding new or ported examples as needed to showcase features and capabilities. In other words, eventually abandon the historical legacy of existing examples; what are good examples for Matplotlib now and in the future?

This was referenced Jan 13, 2017

Member

QuLogic commented Jan 16, 2017

Backported to v2.x as 2f3041e.

vollbier deleted the vollbier:frontpage_html_plots branch Jan 17, 2017

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