Oh, and you can see how it ends up looking by comparing http://eteq.github.io/astropy/modeling/index.html#module-astropy.modeling.models (that's this PR) to http://docs.astropy.org/en/latest/modeling/index.html (current master)

@eteq I agree this is much better. Hope this can make it into 0.3. Thanks for working on this.

@embray - this should be good to go... should I merge this and leave it to you to backport to 0.3.x, or what?

Sorry I didn't get to do much catchup on Astropy yesterday; I can merge things like this any time. I would right now but it seems GitHub is having trouble with some of their fileservers at the moment and it appears to be affecting write access to the repository.

@eteq - maybe I misunderstand, but it looks like as if your proposed changes don't only affect the docs:

We have many many models and some have very short names that are only unambiguously meaningful within a context. For example, the legendre polynomials (1D) reside inside the astropy.modeling.polynomial namespace thus being astropy.modeling.polynomial.Legendre1D. This shows the user that we're talking about the Legendre polynomials and not the legendre transformations.

As we have so many models, I think we really should keep hierarchy in the code itself (don't know about the docs).

I don't see that there's a whole lot of ambiguity in this even as it is, though if there is we could always later rename Legendre1D to LegendrePolynomial1D.

@embray: I'm saying that the current state is good - do not change.

@wkerzendorf - this has not changed anything in the code (except for the sphinx extensions). In the current code one can already do from astropy.modeling.models import Legendre1D. One can also do from astropy.modeling.polynomial import Legendre1D. That's because the class is implemented in astropy/modeling/polynomial.py, but astropy/modeling/models.py provides a location for all of the models.

The point is that you want a central place for the models to be visible, but don't want it in two different places. There was a lengthy discussion months ago that settled on the from astropy.modeling.models import whatever form. This is mainly rectifying the original sphinx extension problem that prevented it from documented like this in the first place.

I agree with where this is going in principle [in particular I like that we can use automodsumm on sort of "namespace modules" like astropy.modeling.models], but now that I look at the example build I don't think this really presents an improvement yet. Right now all the model classes are listed in alphabetical order and are not grouped very logically. Also Polynomial2D is showing up twice in the list, which is just odd. I'm in favor of postponing this to 0.3.1.

Ok, given that this needs more discussion, let's indeed postpone this to 0.3.1. This is certainly not critical for the 0.3.0 release.

Also, I disagree that any of them are more ambiguous this way that before. Some (like Beta1D) are just always ambiguous because of the conventions used in the field, but it's clear they're all models. I could see the argument for Legendre1D, but I think the better solution would be to change that particular one to LegendrePoly1D if we get any reports of people confusing the transform with the polynomial. So that was why the astropy.modeling.models module was made in the first place.

Right--it's still coming from the modeling package so even if we did have a legendre transformation function (and it were class for some reason) the only ambiguity would arise would be if they're both used in the same module.

@eteq sorry, couldn't see the code, because somehow github doesn't load it. Anyways, I disagree with making them all visible under astropy.modeling.models (because I believe namespaces are one honking great idea ;-) ), but that discussion has seemingly passed and this PR only rectifies some leftover changes.

Hmm, yeah, the Polynomial2D being used twice is indeed odd. So I see that this should be delayed.

Are we willing to include it in 0.3.1 even though nominally it introduces a new feature (the extra option in automodapi?) That's necessary for making this work. I don't mind, but I just want to make sure that's not going to bother anyone else. If it is, I would say it would be better to pull in just the sphinx part now, on the theory that future bits can be bugfixes.

@wkerzendorf - well, we do have a big red warning that says "modeling could change in the next version" in the docs for 0.3, so it certainly can be changed if you can rally support :)

regarding the next discussion of ambiguity: the legendre transformation would be a model (if one were to write it) and it is only one example of this going wrong. We will have many many models and I think namespaces clashes will be really annoying.

@eteq I'll just bide my time for the first namespace clash - then I rally support 😉. For now it's fine and we have bigger fish to fry.

@embray @astrofrog @wkerzendorf - I split out the sphinx stuff in #1808, and I can rebase this on that for 0.3.1 if we want.

After some discussion in #1808, we opted to just wait on 0.3.1 for both the sphinx tweaks and adjusting the modeling docs.

@nden @astrofrog @wkerzendorf @embray - Now that v0.3 is out, should we go ahead and merge this now for v0.3.1?

Well, 👍 from me but others should weigh in too.

it's fine from me as well

👍

Is there somewhere we can see a build of the docs in this PR? I looked at http://eteq.github.io/astropy/modeling/index.html#module-astropy.modeling.models but it still shows the problems I raised previously? If it's too much trouble I can just try building it myself.

@embray - there is not, but I've been thinking about possible ways of doing this. It should be pretty easy to modify travis to send the built documentation somewhere, but the question of "somewhere" is what has kept me from implementing it.

I had not intended to fix the fact that they are "not grouped logically", as it's not clear to me what the right "logical" order is, because I can think of a few different, conflicting ways of organizing. But you're right that Polynomial2D is still duplicated - I'll have to check on that.

@embray - figured out the duplicate Polynomial2D and fixed in commit da5991b. The problem was that there was an item variable left in the models namespace that was ending the loop in Polynomial2D, but then it got documented separately. Look good to you?

Well, at least many of the model classes are already grouped somewhat logically by what module they're defined in. Not all of them have an obvious taxonomy to them, but for example I think it makes sense that the various polynomial models are grouped together, and so on.

@embray - I see your point... But I personally found the "taxonomy"-based grouping to be confusing, because it doesn't map completely onto how these things logically group together in my head. Of course, that may be just me, but if it happened to me I suspect that means other people have other ways of thinking about these things. So that's why I was thinking it makes sense to not enforce a specific taxonomy.

Do you have a specific idea how you'd like to see this that addreses both the taxonomy problem and the fact that we want them to be accessed from atropy.models? Or should I merge this now, and then we can consider improvements in a subsequent PR (I don't want this to sit too long, because it has a variety of automodapi/automodsumm fixes that should go in...)?

@eteq - one option is to use what @Cadair implemented in #1847 to split the models according to what makes sense from the user point of view and put more user-friendly headings.

clearly this is not happening for 0.4, so I re-milestoned it for 0.4.1 (although it might be better for 1.0, as there will be a lot of modleing changes there, anyway.

@eteq - do you think it makes sense to wait for more modeling work to happen, or can we rebase and merge this soon?

@astrofrog - I'm not sure what pending changes are affecting this... (@nden or @embray maybe know?)

But it's definitely not ready to rebase and merge, because it's still waiting for other people to say yea or nay on my last proposal (see the Jan 16 comment).

I just looked at the current devdocs. The place where I expected to find all models is at the bottom of the modeling page in the docs and I currently see all of them there. So, is this issue still relevant? Should the "other parts" mentioned by @eteq above be pulled out to a separate PR?

It's relevant in that it could still use better organizing, I think. I've made some changes recently that moved things around a bit better. But it's true that I think most of the code in this PR is no longer relevant and if any of it is worth keeping (is it?) it needs to be merged into astropy-helpers instead.

@eteq Just going over some old issues. As far as I can tell this is almost (if not cpompleteley) identical to the current docs. Should we close this PR? I think the modeling documentation is due for another round of updates but perhaps it should be started from scratch?

Closing as @nden suggested.

Wow @pllim, you did a lot of closing while we were having dinner over here.

Must... achieve... goal... 😉

Brilliant, I hoped that I could get you on board :)

110 is my personal best today. I can do no more... 😢