This repository was archived by the owner on Nov 11, 2024. It is now read-only.
Repository navigation
This repository was archived by the owner on Nov 11, 2024. It is now read-only.
Move test stuff to astropy-helpers if there's a need for it outside astropy-dependents #73
Description
In APE4 and the current master, the test runner and related framework are not in astropy-helpers, but rather in astropy. The justification from @embray in APE4 is that the tests have to be at runtime, so it makes sense to leave it in astropy given that affiliated packages will need it at runtime anyway.
But while working on #72 I realized an issue with that: the test runner and test command are now in separate packages, so they can't be kept in sync. More generally, it's really confusing to figure out how they interplay when they are in different packages. So maybe we should just move all the testing stuff to astropy-helpers too?
cc @astrofrog
Activity
astrofrog commentedon Aug 8, 2014
@eteq - so just to be clear, you are suggesting removing the ability to do
astropy.test()?eteq commentedon Aug 8, 2014
@astrofrog - oh, no, that should still be possible, but it should use code generated by
astropy-helpers. I'm thinking something like what we do withversion.py, which gets generated at build-time from a single "authoritative" source.embray commentedon Aug 8, 2014
I think updates that affect both ends of things are infrequent enough that it's not worth the effort.
eteq commentedon Aug 8, 2014
That may well be true... but as far as you're concerned, it's a "not worth the effort" problem rather than "it shouldn't be done" issue, @embray?
embray commentedon Aug 8, 2014
No, not even that. I think if anything the degree to which the
setup.pytest command needs to be intimately tied to anything in astropy should be lessened. It already depends on astropy to run, so there's no reason it couldn't get most of the information it needs from the installed astropy version as well.eteq commentedon Aug 8, 2014
I was more thinking about this in the context that
astropy-helperscan be used by packages that don't necessarily have anastropydependency. But those don't exist yet, and perhaps never will, so I guess it's a non-issue right now.[-]Revisit whether test stuff should be in astropy-helpers[/-][+]Move test stuff to astropy-helpers if there's a need for it outside astropy-dependents[/+]eteq commentedon Aug 11, 2014
@embray - I changed the name and set it to "future" on the theory that this is only worth doing if someone wants to use
astropy-helpersbut not necessarily have anastropydependence. Or, if you'd rather keep this repo clean, we can always close it and re-open when it makes sense. Whatever you prefer is fine with me.embray commentedon Aug 11, 2014
Yeah leaving this open is fine. I think there are some things that could be done here to improve the situation.
eteq commentedon Feb 2, 2015
@embray @astrofrog - I wanted to report an oddity related to this issue that I just encountered. Halotools (which uses the package template) was getting strange errors on travis while trying to run their test suite. It turns out this is because they were using numpy v1.8 for the main part of the test suite, and the conda packages for astropy are numpy 1.9 only after v4.1. So they were getting astropy v4.1, which is incompatible with the current astropy helpers (due to the addition of the
repeatkeyword for the test runner). Having all of the test stuff in the helpers would get around such versioning issues.embray commentedon Feb 2, 2015
Interesting...I'll have to take a closer look in a bit. But I feel like when I added the
repeatkeyword I updated the test runner so that it wouldn't use unsupported arguments. In other words, I thought I made a specific point of addressing this kind of issue.embray commentedon Feb 2, 2015
@eteq Aha. It appears that the workaround I added for this was only added to the v0.4.x branch on astropy-helpers, and is not in master. I guess the thinking there was that I was going to do something "better" later. But this solution is still better than nothing so I better copy it over...
eteq commentedon Feb 3, 2015
@embray - ok, that sounds good for that particular issue. Can you just ping me when that happens so I know to have halotools update to that version (they can serve as a test case...)?
The main point of this issue is still relevant because it would have prevented this in the first place. But I guess it's not particularly more urgent now than before...
embray commentedon Feb 3, 2015
It's just too bad I didn't copy the change over--then it would've just worked like I planned =D
Fix for the issue I originally mentioned in astropy#73 (comment); thi…
Fix for the issue I originally mentioned in astropy#73 (comment); thi…
travis: use astropy-dev on numpy-1.8 to work around astropy/astropy-h…
Fix for the issue I originally mentioned in astropy#73 (comment); thi…
Fix for the issue I originally mentioned in astropy#73 (comment); thi…
embray commentedon Nov 20, 2015
Oops--this was addressed by #184 and related. The test stuff is all back in astropy itself. It could still be split out of astropy too if some package wanted to use it without depending on astropy. But in that case it should be its own entity separate from both astropy and astropy-helpers.