-
-
Notifications
You must be signed in to change notification settings - Fork 7.8k
sphinx-exhibit #11936
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
sphinx-exhibit #11936
Conversation
8dacf6f
to
4f74424
Compare
Wow, this looks really promising, even tough I had just a quick glance at it! 👍 |
This looks really neat! I'm curious what's the main difference between this and sphinx-gallery. It seems like this is more lightweight, and also has "deeper" inspection of the classes etc, which is nice! My main question comes from a sustainability perspective. I feel like the #1 rule in open-source is "only create new things when you absolutely have to", so my question is: why create a new package that we'll need to maintain rather than add features to sphinx-gallery, which already has a fairly strong dev community? One of the big reasons for the initial move to sphinx-gallery was to remove matplotlib-specific gallery code that was becoming burdensome, and instead use a community standard that many other packages were using. IMO, we should need a very strong reason to move away from a pre-existing, well-vetted, community-wide tool, so I'm just making sure that we've done our due diligence here. I'm not sure what the matplotlib governance rules are on big tech decisions like this, but this seems like a big enough decision (since built examples and the galleries make up most of the online documentation) to warrant getting feedback from the dev community in a rigorous way. |
cc also @NelleV as she was the main driver of the initial conversion of the docs into sphinx-gallery |
I am glad to hear that you like it. I knew the "why not PR these into SG?" question was coming... :) so here are some thoughts.
To be clear, I think SG is a great project and did a lot to improve the state of docs in the scipy community (I also realize that not everyone has the chance of writing Py3-only code...). SE is just "how I would have done things differently", and some of these differences are just questions of taste (directives vs. conf.py+comments). What happens now? If the SG devs wants to integrate some of the features I wrote for SE (which I guess would mostly be better backrefs and better notebooks), they should definitely go for it (in a sense, you can take SE just as proof that "it can be done"). In other words, if SE can help making SG better, I'm happy too. |
A quick couple of thoughts: I think you're bringing up a lot of reasonable points. I don't think we really need to go into specific technical details here, my points are more about organizational process than technical pros/cons. In general, I think a large and well-maintained project like matplotlib should also have a strong preference for other large or well-maintained projects (for non-trivial code, anyway). For example, the goal of moving away from using the matplotlib gallery directives and towards sphinx-gallery was because we were removing custom code that was understood/maintained by very few people, and instead depending on a well-tested/maintained package that was already fairly well-used and maintained (e.g. in the scikit-learn documentation). I think the decision to move away from an established project and towards a newer/smaller project shouldn't be taken lightly. That said, I'm probably going to put a hold on any sphinx-gallery improvements in matplotlib (such as #11415 ) until we get some clarification on this, since I don't want to waste my time improving stuff if it's going to be replaced later on anyway. I promise this isn't meant as a negative comment, just trying to mindful of time commitments! To be transparent: Obviously I have strong, strong preference for "don't create a new thing, contribute to a pre-existing thing" in this space. There are a million ways to solve these problems from a technical perspective, the hard thing is building sustainable / maintainable communities around the tools. Sometimes it does make sense for the ecosystem to fracture though! |
I share @choldgraf's concern about maintainability. I don't think this approach is sustainable neither in terms of software development nor from a community perspective. I'm also tagging @tacaswell in this discussion. Dependency changes should be discussed with the release manager and core maintainer, even when they only affect the documentation. |
I must say, from my experience on https://matthew-brett.github.com/nb2plots, that I can well imagine that using directives would be more attractive than special comments. There's surely a risk for maintenance, and a benefit from improved notebooks and introspection. So - how big is the maintenance risk? @anntzer - can you commit to maintaining this for - say - 5 years? |
As noted above, I think both of the most important additional features (better introspection and better notebook generation) could be adopted by SG -- I just won't be the one to write that PR at least until SG drops Py2 support and starts tracking sphinx versions much more aggressively (that's not my decision; I am not familiar with SG's internals either but I doubt that is a problem). In fact, I see that @GaelVaroquaux just mentioned having the same idea for notebooks in sphinx-gallery/sphinx-gallery#402. The use of directives to specify the galleries would be a bigger change, both backwards incompatible and, at least a year ago, opposed by a SG core dev (sphinx-gallery/sphinx-gallery#275 (comment) as noted in a previous comment). On the other hand, while I think directives are nicer, gallery order is not something you need to fiddle with very often, and the API is not visible to those who actually read the docs, so the incentive to change is smaller. The same point applies for directives vs. special comments. If SG implemented the additional features (and possibly switched to directives, or not), then there would be no reason to switch to SE. In fact, note that I published this PR as soon as these main points were implemented in SE, so you can think of this as a sort of "technical preview" ("showcase some features", as written at the top of the PR); things such as thumbnails (generation, CSS) are well understood and at least I can't think of major improvements over what SG does (sure, I could just have copypasted/imported SG's code for that, but what's the point? I did import SG for parsing the examples though as that was needed to actually build the Matplotlib docs :-) Or you can think of this PR as a mega-PR of SE into SG, except that I was too lazy to actually write it on top of SG's current implementation and split it into reviewable chunks... Do I want to fragment the community? Of course not. But (while I realize that the discussion can be a source of tension) I do think (given on my unwillingness to write Py2 code) that the community is better off knowing that things that I propose in SE are technically possible (beyond the vaporware state). SE is incomplete: it lacks thumbnail generation, and tests are... limited (though you can run pytest and get pretty good coverage, but it's just smoketesting), so bugs likely present. If we do decide to adopt it (which, as stated above, is far from being obviously the best choice), I would definitely prefer if some other devs familiarized themselves with it (also that gives me an excuse to look for volunteers to finish the parts I was too lazy to implement :-) of course I can go over the implementation with whoever is interested). While I am a priori happy to maintain SE for as long as needed, I also agree that it makes no sense for Matplotlib to start having a dependency on a library with a bus factor of one. |
A few points from my end:
|
Actually, here's another technical point: I just benchmarked SE and it plays quite nicely with sphinx's builtin parallel build (... which doesn't work on Windows but that's sphinx's problem to fix :-)) It is known that sphinx's parallel build doesn't actually scale very well (sphinx-doc/sphinx#4575) but there's still a significant improvement. SE no-parallel: 466s SG no-parallel: 456s Caveat: sphinx complains with a bunch of "image not found" errors during the parallel SE build. This is likely because the some docs refer to some images that haven't been generated yet (because they're in another process) (SG doesn't run into this issue likely because it runs all examples first before reading the docs). But the images it's complaining about are actually present at the end of the build, so I think things are fine (haven't done a careful check though); therefore, I readily admit that the comparison isn't totally fair. Again, you may see this as a nudge/challenge for SG to scale better with the number of cores :-) (In fact, I think that in that specific case, because SG runs the examples outside of sphinx's main "read" loop, it can more easily implement its own, better parallelization scheme.) |
@choldgraf - the requirements seem a bit strict. If SE was obviously better, and feature complete, and @anntzer had committed to maintain it for 5 years, are you really saying you'd insist on two extra core developers before you'd consider it? @anntzer - in my experience, it very rarely works if you have a new project that is nearly feature-complete, and you are waiting for others to come and help finish it up. For SE to be a contender, it would really have to be a better drop-in replacement for SG, at which point the discussion could be the trade-off between size of developer base, code maintainability and features. |
@matthew-brett I wasn't trying to say that this was a strict rule (that's why I said it was semi-arbitrary). Just saying that a large package like Matplotlib should err on the more conservative side in its dependencies, especially when there's already a well-developed package out there that it already uses. Though from a maintainability perspective, it does seem like a reasonable concern. It sounds like the main reasoning that @Titan-C was 👎 on directives was because it was a big pain to maintain. If that's true then it doesn't comfort me that a single person decides they want to maintain this functionality on their own... I also agree with you that maybe it makes sense to adopt a different package if it implements truly crucial functionality that we can't live without, and if we have exhausted attempts at getting these features into the already-used package and this hasn't been successful. My point is that we haven't tried the "contribute to a pre-existing thing" path yet. E.g., I don't believe the sphinx-gallery community has had an explicit conversation about using a directive. I am all for growing out new pieces of the ecosystem if there is a well-thought-out and strong reasoning behind it (e.g. when repo2docker decided to use its own image building pipeline instead of using s2i, we wrote a whole blog post justifying why). I'm trying to make sure we exhaust those options before considering using a totally new tool that replicates much of the functionality of an already-existing and well-maintained tool. |
4f74424
to
2ae01ec
Compare
Thanks for the advice. Given the point it's at, I'll probably finish the implementation myself at some point anyways (though on a yet-to-be-defined timeframe) to make it truly usable, but (as stated above) the main priority was really to convince myself and hopefully others that the additional features are indeed possible.
I don't think the problem is with the maintenance, it's with keeping compatibility with \infty versions of the dependencies. Again, I think that if you can git clone matplotlib/sklearn/... and possibly even build it :-) then you can pip install the latest version of a pure python package. I can even be generous and guarantee to support the last two minor versions of sphinx at any time, that should give you plenty of time to upgrade it.
On a project like this, the energy barrier for me to write something from scratch that only supports the last version of everything (I added Py3.5 support just recently :-)) is much lower than trying to adapt to an existing codebase, especially when I disagree with certain design decisions in that codebase (again, some of it is just matter of taste). Some may think that this is not the most constructive approach (due to the friction it causes), but as stated above I think it is at least better than not writing anything at all and just discussing vaporware.
It is true that there hasn't been an explicit discussion about it on SG's tracker, but I did raise the idea and it was disliked by a core dev and a bit before (same thread) by @larsoner on backcompatibility grounds, even though the feature that would have been broken had been released 1 day before. Perhaps I could have pursued this more (just as I could have opened an issue to ask to switch to Py3 only) but frankly I have little interest in these doing that; I tend to think that at the end it's the implementation that speaks for itself. |
2ae01ec
to
86ce2d6
Compare
> It sounds like the main reasoning that @Titan-C was -1 on directives
> was because it was a big pain to maintain. If that's true then it
> doesn't comfort me that a single person decides they want to
> maintain this functionality on their own...
>
> I don't believe the sphinx-gallery community has had an explicit
> conversation about using a directive.
It is true, at SG we haven't had an explicit conversation about
directives beyond
this: sphinx-gallery/sphinx-gallery#26 (comment)
And then our day to day experience dealing with Sphinx. Sphinx is a
complicated package to deal with and it breaks something on every major
version. Thus we try not to use to much of if, since we have to deal
with some legacy platforms and a large span of versions in the
dependencies.
I don't think the problem is with the maintenance, it's with keeping
compatibility with \infty versions of the dependencies.
That is exactly true, our biggest problem is being able to run on many
systems with many configurations and dealing with a bunch of legacy
systems. But that is some sort of maintenance of the package.
I can even be generous and guarantee to support the last two minor
versions of sphinx at any time, that should give you plenty of time to
upgrade it.
If you have the option to choose your dependencies and your policy on
support, it is a great thing to use recent features of the language and
your dependencies. I also wish for SG to support only up to date
versions of Sphinx and be Py3 only. Is just an option we don't yet have.
|
doc/api/animation_api.rst
Outdated
@@ -19,7 +19,6 @@ The easiest way to make a live animation in matplotlib is to use one of the | |||
|
|||
.. autosummary:: | |||
:toctree: _as_gen | |||
:template: autosummary_inher.rst |
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 seems un-related/un-intended? The inherited members on the Animation classes help quite a bit.
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.
That's from #11931, see reasoning there (tldr: the default template already shows inheritance now).
Built this PR on top of it so that I only have to rewrite one template, not two.
86ce2d6
to
6c0e84c
Compare
6c0e84c
to
17359b7
Compare
17359b7
to
8d1bd9b
Compare
Given this all works now in sphinx-gallery, can this PR be closed? |
As hinted in a few other places in the issue tracker, I have been working on a new example runner for the docs. While it is still incomplete, I think I can now put up this PR to showcase some features.
See https://3905-7439715-gh.circle-artifacts.com/0/home/circleci/project/doc/build/html/index.html for an example build.
A (very) brief README for the sphinx extension (boringly named "sphinx-exhibit") is available at https://github.com/anntzer/sphinx-exhibit
What works, especially in comparison with sphinx-gallery:
ax.plot
ismatplotlib.axes.Axes.plot
. See e.g. the hillshading demo:and by sphinx-exhibit
(Yes, you notice the broken image link more in the latter, but note that the image is actually also missing in the former :))
What remains to be done:
Note that sphinx-exhibit is not on PyPI (yet) so this build installs it from my github repo. Also, I use sphinx-jinja for templating (see doc/gallery/index.rst),
but that project has a few bugs and my PRs are not getting merged (the project seems inactive), so I also install it from my repo (I may ultimately fork that project under a separate name if it remains inactive).(The sphinx-jinja manager merged my PRs and pushed a new version to PyPI, so things are fine now on this front.)Note that the first commit of this PR is #11931.
attn @ImportanceOfBeingErnest @jklymak @timhoffm (who discussed this in #10974) and @choldgraf (as sphinx-gallery dev).