astropy / pyregion

ds9 region parser for python
https://pyregion.readthedocs.io
MIT License
39 stars 40 forks source link

Docs change theme to default astropy #96

Closed bsipocz closed 7 years ago

bsipocz commented 8 years ago

@cdeil - I acknowledge that there are issues with the default theme, but it's rather nice to have a shared look for the packages.

The theme is hosted in astropy-helpers, so any suggestion for improvements are I think welcomed there.

cdeil commented 8 years ago

I'm strongly 👎 on this change.

The docs theme https://github.com/snide/sphinx_rtd_theme has over 1000 stars on github, is used by 1000s of projects and several astropy-affiliated packages. That has a good reason ... a lot of work went into making it, it looks nice on desktop, tablets and phones.

Asides from my personal opinion that the Astropy theme doesn't "look good", it does not work as well and is not as well maintained.

So if anything: please stop changing packages to the Astropy theme, and instead let's change more to a better theme if you think "shared look for packages" is important.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 63.243% when pulling 7fd62fa0a2dae7bef0a0f5f390706812f3f93419 on bsipocz:docs_change_theme into 9c07e37ddcb229486fc21bc28c26f3539c338ee2 on astropy:master.

bsipocz commented 8 years ago

So if anything: please stop changing packages to the Astropy theme, and instead let's change more to a better theme if you think "shared look for packages" is important.

I switched only a small number of packages as it was included in the upgrade to use the latest package template (solving quite a lot of other issues). Obviously accepting these changes are optional. Also many of the astropy maintained packages were already using the theme for quite a long time now.

Also, my view is that it's good to have a different look than the other 1000x packages out there that use sphinx_rtd. Until someone takes the time to figure out how to customize it, whether it is customizable enough, I would rather see packages to share that of astropy so emphasize the connection, rather than to look ok on a phone.

Anyway, it may be worth raising this at the helpers and have a discussion there with others who worked on the theme.

keflavich commented 8 years ago

@cdeil could you open the discussion on theming on package-template? I agree with @bsipocz 's logic that we should have a common rtfd theme or look for astropy-affiliates, and so I support the general approach of enforcing that conformity. However, if there are problems with the theme (....there are), we should be fixing them across the board.

astrofrog commented 8 years ago

I like sphinx-rtd-theme but I also like the idea of having customized themes to make our packages more recognizable. So I think it would be great at some point to fork sphinx-rtd-theme and maybe just change the color theme and add the logo.

cdeil commented 8 years ago

@keflavich wrote:

@cdeil could you open the discussion on theming on package-template? I agree with @bsipocz 's logic that we should have a common rtfd theme or look for astropy-affiliates, and so I support the general approach of enforcing that conformity. However, if there are problems with the theme (....there are), we should be fixing them across the board.

As I said before, I disagree. I'm certainly 👎 on enforcing conformity or even to try and convert projects like this one with nice and perfectly working Sphinx docs to astropy-helpers.

I'd rather not start a discussion issue on astropy-helpers on that topic. We could start a thread on astropy-dev, or just delay that discussion to the next Python in astronomy and have a face-to-face discussion, immediately followed up by sprinting on improving things. IMO that would be better than a GH issue with discussions and (my guess) no action.

Just to give some background where I'm coming from: when astropy-helpers was introduced in 2004 I asked for one page of documentation how it works. In https://github.com/astropy/astropy-helpers/issues/56#issuecomment-49770563 @eteq commented that he agrees and even that some docs explaining how astropy-helpers works are high priority. It's 2016 and I still don't know how the astropy-helpers Sphinx stuff works, every time there's an issue or I try to configure or extend something, it's very frustrating and takes me a lot of time.

bsipocz commented 7 years ago

@astrofrog @cdeil - What do you think, figuring out an astropy theme that builds on sphinx_rtd_theme is more like a hackday project or somewhat longer and could be part of a gsoc project? E.g. do you plan to add the gammapy web dev project again this year? If yes, would you mind to have this piggybacking on it?

cdeil commented 7 years ago

Side comment: For Gammapy, we're doing two face-to-face meetings in the coming weeks (see here) to grow the user and contributor base, and we're competing with Gammapy as CTA science tools this year against Gammalib/ctools. So I probably won't have time to mentor for GSoC at all, unless it's with someone very good that's already contributing.

Apart from that, I'm not sure there's a good GSoC project here. I mean, the first step would be to discuss if Astropy and affiliated packages want a new theme at all, no? To me it's not clear that more than a few people really care enough about improving things at the moment, and in what direction to go (improve current theme, or start new one based on sphinx_rdt_theme).

So my suggestion here would actually to close this PR for now, and then to maybe bring up this question at some future f2f meeting where many Astropy devs / users are present.

cdeil commented 7 years ago

What do we do here? It doesn't help to leave this PR open without a decision.

I prefer the RTD theme we have now (see https://pyregion.readthedocs.io/) and @bsipocz prefers the Astropy theme and this PR makes the change.

Maybe other pyregion devs can vote (cc @leejjoon @keflavich @sargas @astrofrog) and then we decide?

astrofrog commented 7 years ago

I would prefer to defer this until we know what the long-term future of this package is given the regions package.

bsipocz commented 7 years ago

I don't have strong feeling about this repo, so happy to close the PR without merge.

However I still strongly feel that having a uniformish look for (some) of the affiliates are very valuable. We didn't get anywhere at pyastro about customizing the sphinx-theme, but I still have some hopes that @Cadair et all will manage to get something done (or explore the options) as part of the sunpy web gsoc.