APE4: Astropy Setup Helpers - #5

Some general questions/comments:

• Will astropy_helpers be documented in the astropy documentation, or will it have its own documentation? (I'd prefer the former, aside from a README)
• Along the same lines, while it have its own independent issue tracker, or will we direct everyone to astropy? Obviously PRs would have to be in its own repo, but it's possible to turn the issue tracker off... (Again, I'd prefer to keep as much as possible in the astropy repo, to prevent confusion.)
• Two other pieces of astropy that might be in the "helper" category: logging and the configuration system. It's possible they even have to be, if any of the parts that we want to move to be helpers use the configuration items...?

@eteq - the issue with the docs is that then they will be decoupled from the code itself. Since we want astropy_helpers to have a faster release cycle, we also want the associated docs to have the same release cycle, so we have to keep the docs separate. However, we can make it mostly seamless to users/developers if we use the same template, and include a link to it from the main astropy docs. We could have the domain helpers.astropy.org point to the helper docs.

For issues, the problem is that users should hopefully not know about the astropy_helpers package, it should be seamless to them, so they will open issues in the main repo anyway. However, we have to support pull requests for astropy_helpers so we may have to keep an issue tracker there too anyway. I'd vote that we keep an issue tracker for astropy_helpers, but understand that some of the issues will be reported in the main astropy repository, and there isn't much we can do about that.

I agree that the logging and configuration items could be included too. At the end of the day, we're basically separating infrastructure code from functionality, which in my mind is a good idea because they need different release cycles.

At the Developer Hangout today, we discussed this APE and decided a few things. I think this is a list of the key items, altough @astrofrog may have more:

• We want separate docs for astropy and astropy_helpers, but not separate issue trackers.
• As a result, all the sphinx stuff should be in astropy_helpers
• @embray emphasized the importance of astropy_helpers being only build-time and not a run-time requirement
• As a result, configuration and logging should not be in astropy_helpers. Similar for testing, although that may have to be a mix, depending on exactly how we end up supporting python setup.py test.
• The sdist/bdist in astropy_helpers should have an option to include the latest astropy_helpers as part of the tarball when e.g., making an astropy release. When someone installs from that tarball, it will try to get the latest version, but if it can't (or if forced to with an --offline option), it will instead use the astropy_helpers that's in the tarball.
• If there are actual API changes to astropy_helpers, rather than just bugfixes, they will have different version major numbers, so that versions of affiliated packages can be tied to a specific major version of astropy_helpers (but still get an up-to-date bugfix version)

So the APE should be updated to reflect those items.

@eteq - thanks for already summarizing this :) There were only a couple of other things:

• For testing, the idea is that astropy_helpers could implement a non-functional test command so that the command at least is recognized, but an error is given saying that Astropy is required for the tests to run. This was deemed reasonable since most affiliated packages will need astropy installed anyway for the tests to pass.
• The sdist command in astropy_helpers should be make setting up the whole including the latest astropy_helpers in the tarball automated and transparent to the user.
• There was concern as to what would happen if the URL request to get the latest astropy_helpers got stuck, and the solution is to put a message before the download saying that users should run with --offline if this command gets stuck - this forces to use the bundled astropy_helpers and doesn't try and get the latest and greatest.
• We decided that it would make sense to aim for this APE to be decided by the end of February, so that we can give people a deadline for review and time for the implementation to be well tested.

Just as a reminder, we tentatively set a deadline of roughly in a week time to decide on this APE, so it would be good to try and finalize it. Is there anything that I or others can do this week to help try and make sure this stays on track?

cc @embray @eteq

Well, I don't think I've actually done anything on this yet aside from offering opinions... so it'll be good to hear what @embray has to say on the implementation side :)

I've had exactly no time to work on this.

Do you think you might in the near future (i.e., so that we can try to have it for 0.4), @embray, or should we consider shopping around for someone else to work on the implementation with you who might have more time?

No, I'm just focused on getting releases done right now. Then I can turn my attention. I think we'll have that done today.

Ok, sounds good

Finally making some progress on this again. I'm going to make issues for the items brought up in the meeting from January (so glad somebody took notes!) and update the APE as I implement them.

Sounds good!

I don't see much need for the option of including a source tarball of astropy_helpers -- it seems like a hassle to maintain as part of, e.g., an affiliated package. I don't object to it if others find it useful, just can't imagine using it myself.

It sounds like there are also no objections to this APE, so I think we are also good to accept the APE and merge this and the two associated pull requests.

@embray - do you want to start making sure all three pull requests are ready for merging?

cc @eteq @mdboom @embray

I'm still a little hesitant. Has anyone actually tried adapting their affiliated package to this yet?

In any case, it more or less "works" with the package-template so maybe that's good enough for now, with revisions to be made once we get some real world use?

Hmm... good question. @keflavich @cdeil @kbarbary @wkerzendorf or @mwcraig , have any of you tried this in affiliated packages?

I haven't tried this, but if there's multiple pull requests it's kind of hard.
But merging it into master and ironing out issues over the next days / weeks should be fine, no?

@eteq @embray Not yet - I'm still travelling but will try to update TARDIS, when I'm back in ~10 days.

For affiliated packages the only PR that really matters is astropy/package-template#46 as it demonstrates how one might set up a package to use this feature. In fairness, one major piece of this that is still missing is documentation, but I didn't want to start writing docs until there has been more immediate feedback. That said, maybe I should go ahead and start writing docs on how to use this in a project...

@eteq @embray -- not yet...should have time tomorrow.

I'm stuck -- how do I add astropy_helpers to an existing repo as a submodule?

@embray - maybe you can clear this up for @mwcraig , and then add it to the docs?

Question about a point not addressed here (and maybe it shouldn't be addressed in this APE, don't know): If an affiliated package uses six should it use the six bundled with astropy?

I think that's outside the scope of this. But it is a good question. I'm leaning toward yes, just for the deduplication. But it also depends on the range of Astropy versions you need/want to be compatible with, as we've updated the version of six a couple times.

I agree this does not fall within the scope of this PR. I think that the answer is basically (as @embray said) that it depends which features of six you depend on and which astropy versions you want to support.

I can try this with APLpy this evening.

Tried this out tonight, comments in astropy/package-template#46.

Bottom line: seemed to work after a couple changes.

@embray - apart from the issues in the pull requests for the core package and the package template, is this APE ready to merge/accept? I don't think there have been any objections, and the issues that have popped up are likely just bugs that can be fixed? Or is there still the possibility that the implementation design will change a lot?

I read the APE ... there are some things that should be updated before it's merged.
@embray Do you want me to go through and leave comments now or update this PR now that astropy-helpers is active first?

Right, having both issues in astropy-helpers or in astropy with astropy-helpers label could work.

@embray - what's the situation with this PR? Should we merge it now, or do you think you need to make more changes? Of particular importance is finalizing the dicsussion of how to handle issues in astropy-helpers vs astropy.

@embray - should we go ahead and merge this?

I think so. I don't feel comfortable accepting my own APE so if you would like to do it and mark it as accepted that would be good :)

Ok, merging now. Thanks for all your work on this @embray!