Astropy helpers - #46

@embray - travis is failing because the git clone is failing.

Note that this should likely be merged only after APE4 and astropy/astropy#1563 have been merged, since the clone URL will need to be updated (and the commits optionally squashed)

• Currently, line 97 of setup.py is install_requires=['astropy'], but should, I presume, be 'astropy_helpers'? :)
• Not sure if the initial addition of the submodule was supposed to happen automagically, but it didn't. When I manually did git submodule add https://github.com/embray/astropy_helpers.git astropy_helpers the rest worked.
• I assume "work" in this context was supposed to mean that python setup.py install succeeded with no astropy installed...

@embray - I tried this with APLpy and got:

An unexpected error occurred updating the git submodule 'astropy_helpers':
From https://github.com/astrofrog/astropy_helpers
   f852e99..010ea7e  master     -> origin/master
 * [new branch]      try-with-sphinx -> origin/try-with-sphinx
fatal: reference is not a tree: 1a4d6b1bd97ba5279634589f26e2a0c102af40f3
Unable to checkout '1a4d6b1bd97ba5279634589f26e2a0c102af40f3' in submodule path 'astropy_helpers'

when trying to setup.py install. I get the following when listing sub-modules:

$ git submodule
+f852e991725071a1dff80d7328ee4c8ec2d7a2e0 astropy_helpers (heads/master)

Then if I tried adding the submodule as @mwcraig pointed out, I get:

$ git submodule add https://github.com/embray/astropy_helpers.git astropy_helpers
'astropy_helpers' already exists in the index

Any idea what is happening?

By the way, when I merged in your branch, astropy_helpers was created by default (and empty)

@astrofrog -- funny, I almost mentioned this last night but then didn't.

The underlying complaint in the second case is that there is a local copy of astropy_helpers in .git/modules (not sure how it gets there, but it did for both of us) -- that is the astropy_helpers that git is complaining about. @embray, is ah_bootstrap.py making this maybe?

Delete .git/modules/astropy_helpers, then git submodule add https://github.com/embray/astropy_helpers.git astropy_helpers should work.

Probably a good idea to check git status to make sure it doesn't know about the submodule before deleting. If git does see the submodule then do git submodule deinit astropy_helpers first.

Under no circumstances should you do git submodule deinit . -- that makes git very, very confused :)

@mwcraig No, install_requires=['astropy'] is correct. That's another one of those mis-named keywords from setuptools. Really it means "run_requires" and indicates that astropy is a runtime dependency. One should never use install_requires='astropy_helpers' by design--it is not a runtime dependency (setuptools uses "setup_requires" for that).

As for the other issues I'm not sure. I'll have to try walking through the process of setting up an affiliated package to use this myself, and document the process. I already did as much with the package-template but I think I need to do it again. I suspect the trick involves getting the exact steps for setting up the submodule right.

@embray - just to be clear, all I did was take the current APLpy package, then merge in your branch, resolve conflicts, and that's it. I didn't try and set up the sub-module myself, but maybe I was supposed to?

@embray - I just tried with montage-wrapper and had no issues. I will try again with APLpy later this evening.

@embray - I investigated more the issue with APLpy, and I think the issue was due to the fact that there were residual references to previous versions of this PR in some way (or my previous attempts to get it to work before your PR). I started from a clean PR and everything worked perfectly.

@embray - given this, can we go ahead and merge all the astropy-helpers related PRs today? :)

@embray - once you are ready to merge this, and once you update the submodule URL, you'll need to put the http URL for Travis to pass (the ssh clone isn't working at the moment)

Oh, let me try that and see if I can get the tests to pass with Travis.

Cool, this is passing on Travis now. The next step is to merge astropy/astropy-helpers#1 and then update this PR once more so that the submodule points to the main astropy-helpers repo.

Before I merge this PR though should I do anything with the package-template documentation to go along with it? Because the use of astropy_helpers is going to affect, somewhat, how the package-template is used. I think in general the defaults will just work, but maybe it's still worth documenting the different options pertaining to astropy-helpers?

👍 to a brief update of the documentation, though it could be after merging if time is short now.

@embray - I agree with @mwcraig, a little documentation would be nice, but not urgent. Certainly doesn't need to hold back the 0.4 release of astropy.

Okay, astropy/astropy-helpers#1 has been merged so I can update this to repoint the submodule to the main astropy/astropy-helpers. Once that succeeds we can merge this and, well, we'll see how it goes.

Sounds good!

@embray - looks like it needs HTTP, not HTTPS

That makes no sense.

I just screwed up the URL, that's all.

@embray - ah right sorry, I just assumed that because travis seemed to be asking for username/password it interpreted HTTPS as always requiring authentication.

Yeah, GitHub does that instead of returning a 404. This is presumably in order to protect the existence of private repositories, but it's still confusing.

@embray - apart from my comment above, this is good to go.

@embray - actually, as per astropy/astropy#2519, we should update the submodule to ecb29056e257bbb0984d71e16d00abc697e581e2 but not to the latest commit since this breaks Sphinx builds. Could you update the submodule commit here?

Ok, since this is passing, I'm going to merge and then update the astropy-helpers version in a separate pull request.